Changeset View
Standalone View
sys/kern/kern_jail.c
Context not available. | |||||
#include "opt_ddb.h" | #include "opt_ddb.h" | ||||
#include "opt_inet.h" | #include "opt_inet.h" | ||||
#include "opt_inet6.h" | #include "opt_inet6.h" | ||||
#include "opt_pax.h" | |||||
#include <sys/param.h> | #include <sys/param.h> | ||||
#include <sys/types.h> | #include <sys/types.h> | ||||
Context not available. | |||||
#include <sys/sysproto.h> | #include <sys/sysproto.h> | ||||
#include <sys/malloc.h> | #include <sys/malloc.h> | ||||
#include <sys/osd.h> | #include <sys/osd.h> | ||||
#include <sys/pax.h> | |||||
#include <sys/priv.h> | #include <sys/priv.h> | ||||
#include <sys/proc.h> | #include <sys/proc.h> | ||||
#include <sys/taskqueue.h> | #include <sys/taskqueue.h> | ||||
Context not available. | |||||
}; | }; | ||||
MTX_SYSINIT(prison0, &prison0.pr_mtx, "jail mutex", MTX_DEF); | MTX_SYSINIT(prison0, &prison0.pr_mtx, "jail mutex", MTX_DEF); | ||||
#if defined(PAX_ASLR) | |||||
SYSINIT(pax, SI_SUB_PAX, SI_ORDER_MIDDLE, pax_init_prison, (void *) &prison0); | |||||
rwatson: I find myself wondering if it's time for a new prison_init_prison0() sysinit that can take care… | |||||
Not Done Inline ActionsIs that a problem a patch implementing ASLR needs to solve? lattera-gmail.com: Is that a problem a patch implementing ASLR needs to solve? | |||||
Not Done Inline ActionsIt's a problem that the ASLR patch introduces: before ASLR, there isn't an issue with multiple independent sysinits for prison0. rwatson: It's a problem that the ASLR patch introduces: before ASLR, there isn't an issue with multiple… | |||||
Not Done Inline ActionsWhy this is a problem? op: Why this is a problem? | |||||
Not Done Inline ActionsIf there is avoidable non-atomicity in initialising kernel data structures, it's nice to do so. In this case, it might be that we are not yet ready to initialise prison0's ASLR state until well after we are ready to initialise its mutex .. on the other hand, it would be nice to think that prison0 state is fully initialised before something would need to use its mutex. This is an inevitable source of difficulty in bootstrapping the system, but maintaining congruence between bootstrap initialisation of special-case structures (e.g., prison0) and later instances of the same structure (e.g., every other prison structure) is a reasonable goal. rwatson: If there is avoidable non-atomicity in initialising kernel data structures, it's nice to do so. | |||||
Not Done Inline ActionsIn older version of pax_init_prison(struct prison *pr) we acquired the prison lock, and because this it's needed after prison0's mtx initializations. Currently we don't acquire the prison lock and I think it's fine, if we move the pax's prison0 initialization before the prison0's mutex initialization in sysinit priority list. op: In older version of pax_init_prison(struct prison *pr) we acquired the prison lock, and because… | |||||
Not Done Inline ActionsI was sort of pondering a new sysinit (very vaguely) along the lines of: static void mtx_init(&prison0) pax_init(&prison0) } The interesting question is what sysinit ordering to give it: I guess a new SI_SUB_PRISON could fall after SI_SUB_PAX such that PAX's internal state is initialised before the first prison structure uses it? rwatson: I was sort of pondering a new sysinit (very vaguely) along the lines of:
static void… | |||||
#endif | |||||
/* allprison, allprison_racct and lastprid are protected by allprison_lock. */ | /* allprison, allprison_racct and lastprid are protected by allprison_lock. */ | ||||
Not Done Inline ActionsYou define this as a sysinit here and ... imp: You define this as a sysinit here and ... | |||||
struct sx allprison_lock; | struct sx allprison_lock; | ||||
SX_SYSINIT(allprison_lock, &allprison_lock, "allprison"); | SX_SYSINIT(allprison_lock, &allprison_lock, "allprison"); | ||||
Not Done Inline ActionsRemove blank line. rwatson: Remove blank line. | |||||
Context not available. | |||||
goto done_releroot; | goto done_releroot; | ||||
} | } | ||||
#if defined(PAX_ASLR) | |||||
pax_init_prison(pr); | |||||
#endif | |||||
mtx_lock(&pr->pr_mtx); | mtx_lock(&pr->pr_mtx); | ||||
Not Done Inline Actions... explicitly call it here. Why? imp: ... explicitly call it here. Why? | |||||
Not Done Inline ActionsThe sysinit is for jail 0. This call is for when new (child) jails are spun up. lattera-gmail.com: The sysinit is for jail 0. This call is for when new (child) jails are spun up. | |||||
Not Done Inline ActionsThis is fine. They call it for new prisons and this code does not get executed for prison0. mjg: This is fine. They call it for new prisons and this code does not get executed for prison0. | |||||
/* | /* | ||||
* New prisons do not yet have a reference, because we do not | * New prisons do not yet have a reference, because we do not | ||||
Context not available. | |||||
Not Done Inline ActionsRemove blank line. rwatson: Remove blank line. | |||||
Not Done Inline ActionsRemove blank line. rwatson: Remove blank line. |
I find myself wondering if it's time for a new prison_init_prison0() sysinit that can take care of mutex initialisation, etc.