- User Since
- Oct 29 2015, 5:25 PM (124 w, 2 d)
Thu, Mar 8
Wed, Mar 7
I agree with eliminating the pointless uses of the flag; however, I don't understand why we need a CTASSERT to catch this. It doesn't break anything. At most, it seems like it is simply a style violation. And, I don't think CTASSERTs are necessary to enforce style.
Tue, Mar 6
Mon, Mar 5
Feb 15 2018
Address review feedback:
- Add -Wl, wrapping to the LDFLAGS variable.
- Remove an unneeded set of parentheses.
I tested and confirmed that the new code lets gdb set/delete breakpoints.
Feb 14 2018
Add code to handle remote GDB memory writes. (untested)
Feb 13 2018
I think I've addressed the review feedback, and I'm relatively happy with the proposed change now. So, if anyone has been sitting on the sidelines waiting for this to stabilize, now is a good time to jump in with feedback. :-)
If the kernel ever used more than 1G, correctly initialize the right number of PDEs in the direct map.
Implement review feedback:
- Unconditionally set CR0 when writing to memory in DDB.
- Now that DDB breakpoints work, remove the tunable.
Let DDB set/delete breakpoints, as well as generally modify any memory it wants. (If an administrator doesn't trust their users to use this power properly, they don't need to compile DDB support.)
Feb 12 2018
Implement review comments:
- Extend a comment to note that fixups in .text are still permitted until we set CR0.WP.
- Reorder some variable declarations.
Apply the same read-only protection to the portions of the kernel kept in the direct map.
Incorporate review feedback:
- No kernel option.
- Rework the tunable. It now only controls whether pages that contain kernel text are marked read-only. That's the part I think a developer is most likely going to want to disable for debugging purposes. (That also makes the name somewhat easier, maybe.)
- Remove the efer_nxe variable and just use pg_nx to determine whether the BSP supports the NX bit.
In addition to these changes, I would like to make the corresponding pages in the direct map be read-only. Does that seem acceptable?
Feb 9 2018
Address review feedback.
Feb 8 2018
Jan 31 2018
Is this truly an underflow? Or, is it a low number caused by an overflow in one of the earlier calculations? (It might help if you could give a precise example from your testing.)
Jan 10 2018
I think this should be committed after addressing @wblock's comments.
I think this should be committed basically "as is", unless someone raises serious objections in the next week or so.
After addressing @ian's commands, I think this should be committed "as is", unless someone raises serious objections in the next week or so.
I think this should be committed basically "as is" (perhaps with the few tweaks suggested in the review), unless someone raises serious objections in the next week or so.
Jan 9 2018
I think one of the few weaknesses I see is the way the hash result is cached.
It seems like this could use a man page to describe the mechanism. There are some subtleties that are not immediately obvious, such as the way that shared libraries are protected. In addition, the O_VERIFY flag should probably be documented in the open() man page with a pointer to the verified exec man page.
Jan 8 2018
Nov 3 2017
Added a sysctl to control whether KASSERTs are suppressed after a panic.
Nov 2 2017
I ended up posting this for review, since I didn't want to lose the diff. However, while thinking about this more, I realized that there is probably a better way to do this.
Oct 24 2017
Seems like a good idea! See in-line comments.
Oct 19 2017
For what its worth, here is my 2c on this.
Oct 11 2017
I remain concerned about the performance overhead of activating this where not needed. However, I have no performance information to either alleviate or confirm my fears. Has someone done the work to gather performance information to compare a non-VIMAGE kernel to a VIMAGE kernel with a single VNET?
Sep 26 2017
I'm not sure what I think of this change. It seems like an API change again: now EAGAIN always means that pru_send added the bytes instead of freeing them, while other errors are either fatal or mean that the bytes were lost.
Sep 25 2017
I'm not sure I understand the problem this patch is trying to solve. Can you explain the problem in sufficient detail so we can understand how this patch addresses it? Thanks!
Can you explain how these are different? Both functionally set error to so->so_error, and then set so->so_error to 0. If (so) is properly locked, it looks like these should be equivalent. If (so) is not properly locked, then the new formulation is not guaranteed to overcome this deficiency.
Sep 8 2017
Aug 14 2017
Looks good (with one minor nit inline).
Aug 12 2017
Aug 11 2017
At a high level...
Aug 9 2017
Jul 5 2017
Jul 3 2017
Jun 10 2017
Jun 8 2017
Jun 7 2017
May 31 2017
This is so trivial that I plan to commit it within 24 hours unless someone complains.