Remove extra comment line.

Remove blank line.

Could these headers be more sorted?

This header should not be included here. See above.

Begin formatted comments with "/*-" rather than "/*" so that automated formatting tools know not to reformat the comment.

This should be "security.pax.aslr.compat.exec", I suppose.

Good catch.

pax_get_prison() can suffer a race here -- you usually want the thread's prison, not the process's prison.

This will be addressed. Out of curiosity, under what conditions will the thread's prison not be the same as the process's prison?

There are two problems associated with multithreading here:

(1) Dereferencing td->td_proc->p_ucred can suffer race conditions (e.g., dereferencing pointers to freed memory) if the process lock isn't held.

(2) For multithreaded processes, most system calls / traps will acquire a reference to (and cache a pointer to) p->p_ucred as td->td_ucred for the duration of the call so that the credential is consistent over the course of the call. In those cases, td->td_proc->p_ucred may diverge from td->td_ucred for quite extended periods of time. There is also a brief divergence in the thread performing the credential update itself, which will be resolved quickly by an explicit call to cred_update_thread(), or more slowly for calls that don't do this (e.g., not until the next system call/trap starts) -- this is not done for jail calls (although possibly should be).

All these sysctls should have CTFLAF_MPSAFE set.

The numbers _must_ be aligned with the architectural size of pages and superpages. Putting voluntaristic MI values there breaks superpages.

In FreeBSD's pmap and VM code, we find comments like the one found in link 1 (links below). There's also an interesting function called pmap_align_superpage() referenced at link 2. Given those two things, I'm curious to know how ASLR breaks superpage support.

What ideas do you have to make ASLR and superpages to play well together without sacrificing the security of ASLR?

Link 1:
https://github.com/HardenedBSD/hardenedBSD/blob/hardened/current/master/sys/amd64/amd64/pmap.c#L6681

Link 2:
https://github.com/HardenedBSD/hardenedBSD/blob/hardened/current/master/sys/vm/vm_map.c#L1481

can pr really be null here? possibly for kernel threads, but it is unlikely any of them would call here. I suggest to check it and add prison0 to kernel threads if its not already set.

What's up with these calls? Should not all prisons already have pr_pax_set?

Far too much vertical whitespace here and elsewhere in this file.

Remove blank line.

What do you think about hiding this "expert" settings with adding CTLFLAG_SKIP?

And these. And all of the similar bit settings.

Remove blank line.

Remove blank line.

Here, and in a number of places, you replicate the prison0 setting to the global variable. Is there a reason not to just use the prison0 setting elsewhere, and avoid duplication? prison0 is always defined...

Remove blank line.

Remove blank line.

Remove blank line.

Excessive dots :-)

Good catch. I'll fix that and also make it a single-line comment.

Remove blank line.

Remove blank line.

Remove blank line.

Remove blank line.

Race condition per above.

Remove blank line.

Remove blank line.

Remove blank line.

Remove blank line.

Remove blank line.

Race condition.

Remove blank line.

Remove blank line.

Remove blank line.

Remove blank line.

Remove blank line.

Race condition.

Style(9) requires a blank line here. Please check all other functions without local variables to ensure that they also have blank lines.

Will be addressed in the next patch.

Remove blank line.

In general, the kernel does not use the 'bool' type; please use 'int'.

Will be addressed in the next patch.

op@hardenedbsd kern> git grep -A 3 "^bool"
kern_pax_aslr.c:bool
kern_pax_aslr.c-pax_aslr_active(struct proc *p)
kern_pax_aslr.c-{

kern_pax_aslr.c- u_int flags;

subr_bus.c:boolean_t
subr_bus.c-devctl_process_running(void)
subr_bus.c-{

subr_bus.c- return (devsoftc.inuse == 1);

subr_capability.c:bool
subr_capability.c-__cap_rights_is_set(const cap_rights_t *rights, ...)
subr_capability.c-{

subr_capability.c- va_list ap;

subr_capability.c:bool
subr_capability.c-cap_rights_is_valid(const cap_rights_t *rights)
subr_capability.c-{

subr_capability.c- cap_rights_t allrights;

subr_capability.c:bool
subr_capability.c-cap_rights_contains(const cap_rights_t *big, const cap_rights_t *little)
subr_capability.c-{

subr_capability.c- unsigned int i, n;

subr_sfbuf.c:boolean_t
subr_sfbuf.c-sf_buf_process_page(vm_page_t m, void (*cb)(struct sf_buf *))
subr_sfbuf.c-{
subr_sfbuf.c- struct sf_head *hash_list;

Sounds like 'In general' to me.

Remove blank line.

Remove blank line.

Add blank line (for variety).

Remove blank line.

This function name should not begin with '_'.

Are there cases where 'vm' is not the p's vmspace requiring them to be passed separately? If so, please add a comment on why that happens.

Done in next patch.

Remove blank line.

Remove blank line.

Remove blank line.

Remove blank line.

Remove blank line.

Remove blank line.

Remove blank line.

It would be nice to drop all the funky formatting here. The kernel generally uses output along the lines of:

subsystem: warning text

Without brackets, upper-case letters, exclamation marks, etc.

Remove blank line.

Remove blank line.

Remove blank line.

Please remove these printfs, or at least boot them under boot_verbose. We don't need all this information on every boot.