| Message ID | 1483527632-4523-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi all, some remark below... Le mercredi 04 janvier 2017 à 20:00 +0900, Tetsuo Handa a écrit : > ---------- > John Johansen wrote at http://lkml.kernel.org/r/c83a7281-367f-2459-3bcd-f15b64d950a6@canonical.com : > > On 12/20/2016 12:03 PM, Casey Schaufler wrote: > > > Whether or not José moves forward with PTAGS we have identified > > > an issue with the current cred based hooks for AppArmor, TOMOYO > > > and at least one other proposed module. Regardless of the issues > > > of /proc/pid there is work to be done. > > > > > > > I can take a stab at it in a few weeks. While not critical for apparmor > > it would certainly cleanup apparmor's cred handling. > > John might be still busy or offline now. So, here is my implementation. > ---------- > > We switched from "struct task_struct"->security to "struct cred"->security > in Linux 2.6.29. But not all LSM modules were happy with that change. > TOMOYO LSM module is an example which want to use per "struct task_struct" > security blob, for TOMOYO's security context is defined based on "struct > task_struct" rather than "struct cred". AppArmor LSM module is another > example which want to use it, for AppArmor is currently abusing the cred > a little bit to store the change_hat and setexeccon info. Although > security_task_free() hook was revived in Linux 3.4 because Yama LSM module > wanted to release per "struct task_struct" security blob, > security_task_alloc() hook and "struct task_struct"->security field were > not revived. Nowadays, we are getting proposals of lightweight LSM modules > which want to use per "struct task_struct" security blob. PTAGS LSM module > and CaitSith LSM module which are currently under proposal for inclusion > also want to use it. Therefore, it will be time to revive > security_task_alloc() hook. > > We are already allowing multiple concurrent LSM modules (up to one fully > armored module which uses "struct cred"->security field or exclusive hooks > like security_xfrm_state_pol_flow_match(), plus unlimited number of > lightweight modules which do not use "struct cred"->security nor exclusive > hooks) as long as they are built into the kernel. Since multiple LSM > modules might want to use "struct task_struct"->security field, we need to > somehow calculate and allocate enough size for that field. Casey is trying > to calculate and allocate enough size for "struct cred"->security field, > but that approach is not applicable for LKM based LSM modules. On the other > hand, lightweight LSM modules (e.g. Yama) do not always allocate "struct > task_struct"->security field. If we tolerate managing per "struct > task_struct" security blobs outside of "struct task_struct", we don't need > to calculate and allocate enough size for "struct task_struct"->security > field, we can save memory by using address of "struct task_struct" as > a hash key for searching corresponding security blobs, and as a result > we can also allow LKM based LSM modules. Therefore, this patch does not > revive "struct task_struct"->security field. Hashing is an interesting approach for low bandwidth requests. I was more thinking on a struct within task like below: #ifdef CONFIG_SECURITY struct { # ifdef CONFIG_SECURITY_APPARMOR void *apparmor; # endif # ifdef CONFIG_SECURITY_PTAGS void *ptags; # endif } security; #endif It potential defines 2 "security" fields: security.apparmo and security.ptags More below... > It would be possible to remember location in security_hook_heads.task_alloc > list and undo up to the corresponding security_hook_heads.task_free list > when task_alloc failed. But security_task_alloc() unlikely fails, Yama is > safe to call task_free even if security blob was not allocated, and LKM > based LSM modules will anyway have to be prepared for possibility of > calling task_free without corresponding task_alloc call. Therefore, > this patch calls security_task_free() even if security_task_alloc() failed. > > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Cc: John Johansen <john.johansen@canonical.com> > Cc: Paul Moore <paul@paul-moore.com> > Cc: Stephen Smalley <sds@tycho.nsa.gov> > Cc: Eric Paris <eparis@parisplace.org> > Cc: Casey Schaufler <casey@schaufler-ca.com> > Cc: Kees Cook <keescook@chromium.org> > Cc: James Morris <james.l.morris@oracle.com> > Cc: Serge E. Hallyn <serge@hallyn.com> > Cc: José Bollo <jobol@nonadev.net> > --- > include/linux/lsm_hooks.h | 14 +++++++++++++- > include/linux/security.h | 6 ++++++ > kernel/fork.c | 4 ++++ > security/security.c | 15 +++++++++++++++ > 4 files changed, 38 insertions(+), 1 deletion(-) > > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > index 0dde959..efa1354d 100644 > --- a/include/linux/lsm_hooks.h > +++ b/include/linux/lsm_hooks.h > @@ -534,8 +534,18 @@ > * manual page for definitions of the @clone_flags. > * @clone_flags contains the flags indicating what should be shared. > * Return 0 if permission is granted. > + * @task_alloc: > + * @task task being allocated. > + * Handle allocation of task-related resources. Note that task_free is > + * called even if task_alloc failed. This means that all task_free users > + * have to be prepared for task_free being called without corresponding > + * task_alloc call. Since the address of @task is guaranteed to remain > + * unchanged between task_alloc call and task_free call, task_free users > + * can use the address of @task for checking whether task_free is called > + * without corresponding task_alloc call. Is it possible to add a comment on the state of the task: is it fully initialised? parly only? not initialised at all? More below... > + * Returns a zero on success, negative values on failure. > * @task_free: > - * @task task being freed > + * @task task about to be freed. > * Handle release of task-related resources. (Note that this can be called > * from interrupt context.) > * @cred_alloc_blank: > @@ -1479,6 +1489,7 @@ > int (*file_open)(struct file *file, const struct cred *cred); > > int (*task_create)(unsigned long clone_flags); > + int (*task_alloc)(struct task_struct *task); I suggest to add the 'clone_flags' as below int (*task_alloc)( struct task_struct *task, unsigned long clone_flags); It would allow to treat CLONE_THREAD and/or CLONE_NEW... in a specific way. Obviously, the cloned task is 'current'. This is an implicit argument because it will probably be used by hooks to initialise 'task'. That's all Best regards José > void (*task_free)(struct task_struct *task); > int (*cred_alloc_blank)(struct cred *cred, gfp_t gfp); > void (*cred_free)(struct cred *cred); > @@ -1744,6 +1755,7 @@ struct security_hook_heads { > struct list_head file_receive; > struct list_head file_open; > struct list_head task_create; > + struct list_head task_alloc; > struct list_head task_free; > struct list_head cred_alloc_blank; > struct list_head cred_free; > diff --git a/include/linux/security.h b/include/linux/security.h > index f4ebac1..d7cb449 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -305,6 +305,7 @@ int security_file_send_sigiotask(struct task_struct *tsk, > int security_file_receive(struct file *file); > int security_file_open(struct file *file, const struct cred *cred); > int security_task_create(unsigned long clone_flags); > +int security_task_alloc(struct task_struct *task); > void security_task_free(struct task_struct *task); > int security_cred_alloc_blank(struct cred *cred, gfp_t gfp); > void security_cred_free(struct cred *cred); > @@ -857,6 +858,11 @@ static inline int security_task_create(unsigned long clone_flags) > return 0; > } > > +static inline int security_task_alloc(struct task_struct *task) > +{ > + return 0; > +} > + > static inline void security_task_free(struct task_struct *task) > { } > > diff --git a/kernel/fork.c b/kernel/fork.c > index 11c5c8a..9094a48 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -1643,6 +1643,9 @@ static __latent_entropy struct task_struct *copy_process( > goto bad_fork_cleanup_perf; > /* copy all the process information */ > shm_init_task(p); > + retval = security_task_alloc(p); > + if (retval) > + goto bad_fork_cleanup_audit; > retval = copy_semundo(clone_flags, p); > if (retval) > goto bad_fork_cleanup_audit; > @@ -1862,6 +1865,7 @@ static __latent_entropy struct task_struct *copy_process( > exit_sem(p); > bad_fork_cleanup_audit: > audit_free(p); > + security_task_free(p); > bad_fork_cleanup_perf: > perf_event_free_task(p); > bad_fork_cleanup_policy: > diff --git a/security/security.c b/security/security.c > index 32052f5..b746f85 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -892,6 +892,20 @@ int security_task_create(unsigned long clone_flags) > return call_int_hook(task_create, 0, clone_flags); > } > > +int security_task_alloc(struct task_struct *task) > +{ > + /* > + * Since multiple LSMs might supply this call, we might end up partial > + * allocation. Fortunately, we can unconditionally call > + * security_task_free() if security_task_alloc() failed because Yama > + * (the only user of task_free call as of revival of task_alloc call) > + * is prepared for being called unconditionally and we can as well make > + * new users of task_free call be prepared for being called > + * unconditionally. > + */ > + return call_int_hook(task_alloc, 0, task); > +} > + > void security_task_free(struct task_struct *task) > { > call_void_hook(task_free, task); > @@ -1731,6 +1745,7 @@ struct security_hook_heads security_hook_heads = { > .file_receive = LIST_HEAD_INIT(security_hook_heads.file_receive), > .file_open = LIST_HEAD_INIT(security_hook_heads.file_open), > .task_create = LIST_HEAD_INIT(security_hook_heads.task_create), > + .task_alloc = LIST_HEAD_INIT(security_hook_heads.task_alloc), > .task_free = LIST_HEAD_INIT(security_hook_heads.task_free), > .cred_alloc_blank = > LIST_HEAD_INIT(security_hook_heads.cred_alloc_blank), -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 1/4/2017 3:00 AM, Tetsuo Handa wrote: > ---------- > John Johansen wrote at http://lkml.kernel.org/r/c83a7281-367f-2459-3bcd-f15b64d950a6@canonical.com : >> On 12/20/2016 12:03 PM, Casey Schaufler wrote: >>> Whether or not José moves forward with PTAGS we have identified >>> an issue with the current cred based hooks for AppArmor, TOMOYO >>> and at least one other proposed module. Regardless of the issues >>> of /proc/pid there is work to be done. >>> >> I can take a stab at it in a few weeks. While not critical for apparmor >> it would certainly cleanup apparmor's cred handling. > John might be still busy or offline now. So, here is my implementation. Thank you! > ---------- > > We switched from "struct task_struct"->security to "struct cred"->security > in Linux 2.6.29. But not all LSM modules were happy with that change. > TOMOYO LSM module is an example which want to use per "struct task_struct" > security blob, for TOMOYO's security context is defined based on "struct > task_struct" rather than "struct cred". AppArmor LSM module is another > example which want to use it, for AppArmor is currently abusing the cred > a little bit to store the change_hat and setexeccon info. Although > security_task_free() hook was revived in Linux 3.4 because Yama LSM module > wanted to release per "struct task_struct" security blob, > security_task_alloc() hook and "struct task_struct"->security field were > not revived. Nowadays, we are getting proposals of lightweight LSM modules > which want to use per "struct task_struct" security blob. PTAGS LSM module > and CaitSith LSM module which are currently under proposal for inclusion > also want to use it. Therefore, it will be time to revive > security_task_alloc() hook. I think you've made a pretty clear argument. > We are already allowing multiple concurrent LSM modules (up to one fully > armored module which uses "struct cred"->security field or exclusive hooks > like security_xfrm_state_pol_flow_match(), plus unlimited number of > lightweight modules which do not use "struct cred"->security nor exclusive > hooks) as long as they are built into the kernel. Since multiple LSM > modules might want to use "struct task_struct"->security field, we need to > somehow calculate and allocate enough size for that field. Casey is trying > to calculate and allocate enough size for "struct cred"->security field, > but that approach is not applicable for LKM based LSM modules. On the other > hand, lightweight LSM modules (e.g. Yama) do not always allocate "struct > task_struct"->security field. If we tolerate managing per "struct > task_struct" security blobs outside of "struct task_struct", we don't need > to calculate and allocate enough size for "struct task_struct"->security > field, we can save memory by using address of "struct task_struct" as > a hash key for searching corresponding security blobs, and as a result > we can also allow LKM based LSM modules. Therefore, this patch does not > revive "struct task_struct"->security field. While I agree with your conclusion that it's unnecessary to revive the security field in the task structure now, I think we are going to want it in the not too distant future. We can leave that for the first module that uses it, or we can start the inevitable fight with the owner of the task structure here. I also believe that the infrastructure managed allocation model can accommodate dynamic loading. I have not proposed that yet as it does complicate things somewhat, and the slope is already steep enough. As for hashes and IDs, I hates 'em to pieces. > It would be possible to remember location in security_hook_heads.task_alloc > list and undo up to the corresponding security_hook_heads.task_free list > when task_alloc failed. But security_task_alloc() unlikely fails, Yama is > safe to call task_free even if security blob was not allocated, and LKM > based LSM modules will anyway have to be prepared for possibility of > calling task_free without corresponding task_alloc call. Therefore, > this patch calls security_task_free() even if security_task_alloc() failed. Yes, that is an implication of having a free hook where the alloc hook might fail or might never have been called. In the infrastructure managed case it would require some level of bookkeeping to ensure that the module code can determine if the blob includes data for that module. I have a design for doing that. I don't plan to propose it this time around, but I am sure that I'm doing nothing to preclude it. > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Cc: John Johansen <john.johansen@canonical.com> > Cc: Paul Moore <paul@paul-moore.com> > Cc: Stephen Smalley <sds@tycho.nsa.gov> > Cc: Eric Paris <eparis@parisplace.org> > Cc: Casey Schaufler <casey@schaufler-ca.com> > Cc: Kees Cook <keescook@chromium.org> > Cc: James Morris <james.l.morris@oracle.com> > Cc: Serge E. Hallyn <serge@hallyn.com> > Cc: José Bollo <jobol@nonadev.net> > --- > include/linux/lsm_hooks.h | 14 +++++++++++++- > include/linux/security.h | 6 ++++++ > kernel/fork.c | 4 ++++ > security/security.c | 15 +++++++++++++++ > 4 files changed, 38 insertions(+), 1 deletion(-) I would expect to have at least one module that uses the revived hook included. > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > index 0dde959..efa1354d 100644 > --- a/include/linux/lsm_hooks.h > +++ b/include/linux/lsm_hooks.h > @@ -534,8 +534,18 @@ > * manual page for definitions of the @clone_flags. > * @clone_flags contains the flags indicating what should be shared. > * Return 0 if permission is granted. > + * @task_alloc: > + * @task task being allocated. > + * Handle allocation of task-related resources. Note that task_free is > + * called even if task_alloc failed. This means that all task_free users > + * have to be prepared for task_free being called without corresponding > + * task_alloc call. Since the address of @task is guaranteed to remain > + * unchanged between task_alloc call and task_free call, task_free users > + * can use the address of @task for checking whether task_free is called > + * without corresponding task_alloc call. I don't care much for suggesting that the @task be used in this way. How about deleting this sentence? > + * Returns a zero on success, negative values on failure. > * @task_free: > - * @task task being freed > + * @task task about to be freed. > * Handle release of task-related resources. (Note that this can be called > * from interrupt context.) > * @cred_alloc_blank: > @@ -1479,6 +1489,7 @@ > int (*file_open)(struct file *file, const struct cred *cred); > > int (*task_create)(unsigned long clone_flags); > + int (*task_alloc)(struct task_struct *task); > void (*task_free)(struct task_struct *task); > int (*cred_alloc_blank)(struct cred *cred, gfp_t gfp); > void (*cred_free)(struct cred *cred); > @@ -1744,6 +1755,7 @@ struct security_hook_heads { > struct list_head file_receive; > struct list_head file_open; > struct list_head task_create; > + struct list_head task_alloc; > struct list_head task_free; > struct list_head cred_alloc_blank; > struct list_head cred_free; > diff --git a/include/linux/security.h b/include/linux/security.h > index f4ebac1..d7cb449 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -305,6 +305,7 @@ int security_file_send_sigiotask(struct task_struct *tsk, > int security_file_receive(struct file *file); > int security_file_open(struct file *file, const struct cred *cred); > int security_task_create(unsigned long clone_flags); > +int security_task_alloc(struct task_struct *task); > void security_task_free(struct task_struct *task); > int security_cred_alloc_blank(struct cred *cred, gfp_t gfp); > void security_cred_free(struct cred *cred); > @@ -857,6 +858,11 @@ static inline int security_task_create(unsigned long clone_flags) > return 0; > } > > +static inline int security_task_alloc(struct task_struct *task) > +{ > + return 0; > +} > + > static inline void security_task_free(struct task_struct *task) > { } > > diff --git a/kernel/fork.c b/kernel/fork.c > index 11c5c8a..9094a48 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -1643,6 +1643,9 @@ static __latent_entropy struct task_struct *copy_process( > goto bad_fork_cleanup_perf; > /* copy all the process information */ > shm_init_task(p); > + retval = security_task_alloc(p); > + if (retval) > + goto bad_fork_cleanup_audit; > retval = copy_semundo(clone_flags, p); > if (retval) > goto bad_fork_cleanup_audit; > @@ -1862,6 +1865,7 @@ static __latent_entropy struct task_struct *copy_process( > exit_sem(p); > bad_fork_cleanup_audit: > audit_free(p); > + security_task_free(p); > bad_fork_cleanup_perf: > perf_event_free_task(p); > bad_fork_cleanup_policy: > diff --git a/security/security.c b/security/security.c > index 32052f5..b746f85 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -892,6 +892,20 @@ int security_task_create(unsigned long clone_flags) > return call_int_hook(task_create, 0, clone_flags); > } > > +int security_task_alloc(struct task_struct *task) > +{ > + /* > + * Since multiple LSMs might supply this call, we might end up partial > + * allocation. Fortunately, we can unconditionally call > + * security_task_free() if security_task_alloc() failed because Yama > + * (the only user of task_free call as of revival of task_alloc call) > + * is prepared for being called unconditionally and we can as well make > + * new users of task_free call be prepared for being called > + * unconditionally. > + */ Again, I don't like over-explaining the current behavior in light of proposed but still uncertain change. > + return call_int_hook(task_alloc, 0, task); > +} > + > void security_task_free(struct task_struct *task) > { > call_void_hook(task_free, task); > @@ -1731,6 +1745,7 @@ struct security_hook_heads security_hook_heads = { > .file_receive = LIST_HEAD_INIT(security_hook_heads.file_receive), > .file_open = LIST_HEAD_INIT(security_hook_heads.file_open), > .task_create = LIST_HEAD_INIT(security_hook_heads.task_create), > + .task_alloc = LIST_HEAD_INIT(security_hook_heads.task_alloc), > .task_free = LIST_HEAD_INIT(security_hook_heads.task_free), > .cred_alloc_blank = > LIST_HEAD_INIT(security_hook_heads.cred_alloc_blank), -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 1/4/2017 4:21 AM, José Bollo wrote: > Hi all, > > some remark below... > > Le mercredi 04 janvier 2017 à 20:00 +0900, Tetsuo Handa a écrit : >> ---------- >> John Johansen wrote at http://lkml.kernel.org/r/c83a7281-367f-2459-3bcd-f15b64d950a6@canonical.com : >>> On 12/20/2016 12:03 PM, Casey Schaufler wrote: >>>> Whether or not José moves forward with PTAGS we have identified >>>> an issue with the current cred based hooks for AppArmor, TOMOYO >>>> and at least one other proposed module. Regardless of the issues >>>> of /proc/pid there is work to be done. >>>> >>> I can take a stab at it in a few weeks. While not critical for apparmor >>> it would certainly cleanup apparmor's cred handling. >> John might be still busy or offline now. So, here is my implementation. >> ---------- >> >> We switched from "struct task_struct"->security to "struct cred"->security >> in Linux 2.6.29. But not all LSM modules were happy with that change. >> TOMOYO LSM module is an example which want to use per "struct task_struct" >> security blob, for TOMOYO's security context is defined based on "struct >> task_struct" rather than "struct cred". AppArmor LSM module is another >> example which want to use it, for AppArmor is currently abusing the cred >> a little bit to store the change_hat and setexeccon info. Although >> security_task_free() hook was revived in Linux 3.4 because Yama LSM module >> wanted to release per "struct task_struct" security blob, >> security_task_alloc() hook and "struct task_struct"->security field were >> not revived. Nowadays, we are getting proposals of lightweight LSM modules >> which want to use per "struct task_struct" security blob. PTAGS LSM module >> and CaitSith LSM module which are currently under proposal for inclusion >> also want to use it. Therefore, it will be time to revive >> security_task_alloc() hook. >> >> We are already allowing multiple concurrent LSM modules (up to one fully >> armored module which uses "struct cred"->security field or exclusive hooks >> like security_xfrm_state_pol_flow_match(), plus unlimited number of >> lightweight modules which do not use "struct cred"->security nor exclusive >> hooks) as long as they are built into the kernel. Since multiple LSM >> modules might want to use "struct task_struct"->security field, we need to >> somehow calculate and allocate enough size for that field. Casey is trying >> to calculate and allocate enough size for "struct cred"->security field, >> but that approach is not applicable for LKM based LSM modules. On the other >> hand, lightweight LSM modules (e.g. Yama) do not always allocate "struct >> task_struct"->security field. If we tolerate managing per "struct >> task_struct" security blobs outside of "struct task_struct", we don't need >> to calculate and allocate enough size for "struct task_struct"->security >> field, we can save memory by using address of "struct task_struct" as >> a hash key for searching corresponding security blobs, and as a result >> we can also allow LKM based LSM modules. Therefore, this patch does not >> revive "struct task_struct"->security field. > Hashing is an interesting approach for low bandwidth requests. > > I was more thinking on a struct within task like below: > > #ifdef CONFIG_SECURITY > struct { > # ifdef CONFIG_SECURITY_APPARMOR > void *apparmor; > # endif > # ifdef CONFIG_SECURITY_PTAGS > void *ptags; > # endif > } security; > #endif > > It potential defines 2 "security" fields: security.apparmo and security.ptags This is not going to be popular with distributions like Ubuntu that compile in all security modules. You're usually going to have wasted space in the task structure doing this. > More below... > >> It would be possible to remember location in security_hook_heads.task_alloc >> list and undo up to the corresponding security_hook_heads.task_free list >> when task_alloc failed. But security_task_alloc() unlikely fails, Yama is >> safe to call task_free even if security blob was not allocated, and LKM >> based LSM modules will anyway have to be prepared for possibility of >> calling task_free without corresponding task_alloc call. Therefore, >> this patch calls security_task_free() even if security_task_alloc() failed. >> >> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> >> Cc: John Johansen <john.johansen@canonical.com> >> Cc: Paul Moore <paul@paul-moore.com> >> Cc: Stephen Smalley <sds@tycho.nsa.gov> >> Cc: Eric Paris <eparis@parisplace.org> >> Cc: Casey Schaufler <casey@schaufler-ca.com> >> Cc: Kees Cook <keescook@chromium.org> >> Cc: James Morris <james.l.morris@oracle.com> >> Cc: Serge E. Hallyn <serge@hallyn.com> >> Cc: José Bollo <jobol@nonadev.net> >> --- >> include/linux/lsm_hooks.h | 14 +++++++++++++- >> include/linux/security.h | 6 ++++++ >> kernel/fork.c | 4 ++++ >> security/security.c | 15 +++++++++++++++ >> 4 files changed, 38 insertions(+), 1 deletion(-) >> >> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h >> index 0dde959..efa1354d 100644 >> --- a/include/linux/lsm_hooks.h >> +++ b/include/linux/lsm_hooks.h >> @@ -534,8 +534,18 @@ >> * manual page for definitions of the @clone_flags. >> * @clone_flags contains the flags indicating what should be shared. >> * Return 0 if permission is granted. >> + * @task_alloc: >> + * @task task being allocated. >> + * Handle allocation of task-related resources. Note that task_free is >> + * called even if task_alloc failed. This means that all task_free users >> + * have to be prepared for task_free being called without corresponding >> + * task_alloc call. Since the address of @task is guaranteed to remain >> + * unchanged between task_alloc call and task_free call, task_free users >> + * can use the address of @task for checking whether task_free is called >> + * without corresponding task_alloc call. > Is it possible to add a comment on the state of the task: is it fully > initialised? parly only? not initialised at all? > > More below... > >> + * Returns a zero on success, negative values on failure. >> * @task_free: >> - * @task task being freed >> + * @task task about to be freed. >> * Handle release of task-related resources. (Note that this can be called >> * from interrupt context.) >> * @cred_alloc_blank: >> @@ -1479,6 +1489,7 @@ >> int (*file_open)(struct file *file, const struct cred *cred); >> >> int (*task_create)(unsigned long clone_flags); >> + int (*task_alloc)(struct task_struct *task); > I suggest to add the 'clone_flags' as below > > int (*task_alloc)( > struct task_struct *task, > unsigned long clone_flags); > > It would allow to treat CLONE_THREAD and/or CLONE_NEW... in a specific > way. > > Obviously, the cloned task is 'current'. This is an implicit argument > because it will probably be used by hooks to initialise 'task'. > > That's all > > Best regards > José > >> void (*task_free)(struct task_struct *task); >> int (*cred_alloc_blank)(struct cred *cred, gfp_t gfp); >> void (*cred_free)(struct cred *cred); >> @@ -1744,6 +1755,7 @@ struct security_hook_heads { >> struct list_head file_receive; >> struct list_head file_open; >> struct list_head task_create; >> + struct list_head task_alloc; >> struct list_head task_free; >> struct list_head cred_alloc_blank; >> struct list_head cred_free; >> diff --git a/include/linux/security.h b/include/linux/security.h >> index f4ebac1..d7cb449 100644 >> --- a/include/linux/security.h >> +++ b/include/linux/security.h >> @@ -305,6 +305,7 @@ int security_file_send_sigiotask(struct task_struct *tsk, >> int security_file_receive(struct file *file); >> int security_file_open(struct file *file, const struct cred *cred); >> int security_task_create(unsigned long clone_flags); >> +int security_task_alloc(struct task_struct *task); >> void security_task_free(struct task_struct *task); >> int security_cred_alloc_blank(struct cred *cred, gfp_t gfp); >> void security_cred_free(struct cred *cred); >> @@ -857,6 +858,11 @@ static inline int security_task_create(unsigned long clone_flags) >> return 0; >> } >> >> +static inline int security_task_alloc(struct task_struct *task) >> +{ >> + return 0; >> +} >> + >> static inline void security_task_free(struct task_struct *task) >> { } >> >> diff --git a/kernel/fork.c b/kernel/fork.c >> index 11c5c8a..9094a48 100644 >> --- a/kernel/fork.c >> +++ b/kernel/fork.c >> @@ -1643,6 +1643,9 @@ static __latent_entropy struct task_struct *copy_process( >> goto bad_fork_cleanup_perf; >> /* copy all the process information */ >> shm_init_task(p); >> + retval = security_task_alloc(p); >> + if (retval) >> + goto bad_fork_cleanup_audit; >> retval = copy_semundo(clone_flags, p); >> if (retval) >> goto bad_fork_cleanup_audit; >> @@ -1862,6 +1865,7 @@ static __latent_entropy struct task_struct *copy_process( >> exit_sem(p); >> bad_fork_cleanup_audit: >> audit_free(p); >> + security_task_free(p); >> bad_fork_cleanup_perf: >> perf_event_free_task(p); >> bad_fork_cleanup_policy: >> diff --git a/security/security.c b/security/security.c >> index 32052f5..b746f85 100644 >> --- a/security/security.c >> +++ b/security/security.c >> @@ -892,6 +892,20 @@ int security_task_create(unsigned long clone_flags) >> return call_int_hook(task_create, 0, clone_flags); >> } >> >> +int security_task_alloc(struct task_struct *task) >> +{ >> + /* >> + * Since multiple LSMs might supply this call, we might end up partial >> + * allocation. Fortunately, we can unconditionally call >> + * security_task_free() if security_task_alloc() failed because Yama >> + * (the only user of task_free call as of revival of task_alloc call) >> + * is prepared for being called unconditionally and we can as well make >> + * new users of task_free call be prepared for being called >> + * unconditionally. >> + */ >> + return call_int_hook(task_alloc, 0, task); >> +} >> + >> void security_task_free(struct task_struct *task) >> { >> call_void_hook(task_free, task); >> @@ -1731,6 +1745,7 @@ struct security_hook_heads security_hook_heads = { >> .file_receive = LIST_HEAD_INIT(security_hook_heads.file_receive), >> .file_open = LIST_HEAD_INIT(security_hook_heads.file_open), >> .task_create = LIST_HEAD_INIT(security_hook_heads.task_create), >> + .task_alloc = LIST_HEAD_INIT(security_hook_heads.task_alloc), >> .task_free = LIST_HEAD_INIT(security_hook_heads.task_free), >> .cred_alloc_blank = >> LIST_HEAD_INIT(security_hook_heads.cred_alloc_blank), > -- > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/04/2017 09:48 AM, Casey Schaufler wrote: > On 1/4/2017 4:21 AM, José Bollo wrote: >> Hi all, >> >> some remark below... >> >> Le mercredi 04 janvier 2017 à 20:00 +0900, Tetsuo Handa a écrit : >>> ---------- >>> John Johansen wrote at http://lkml.kernel.org/r/c83a7281-367f-2459-3bcd-f15b64d950a6@canonical.com : >>>> On 12/20/2016 12:03 PM, Casey Schaufler wrote: >>>>> Whether or not José moves forward with PTAGS we have identified >>>>> an issue with the current cred based hooks for AppArmor, TOMOYO >>>>> and at least one other proposed module. Regardless of the issues >>>>> of /proc/pid there is work to be done. >>>>> >>>> I can take a stab at it in a few weeks. While not critical for apparmor >>>> it would certainly cleanup apparmor's cred handling. >>> John might be still busy or offline now. So, here is my implementation. >>> ---------- >>> >>> We switched from "struct task_struct"->security to "struct cred"->security >>> in Linux 2.6.29. But not all LSM modules were happy with that change. >>> TOMOYO LSM module is an example which want to use per "struct task_struct" >>> security blob, for TOMOYO's security context is defined based on "struct >>> task_struct" rather than "struct cred". AppArmor LSM module is another >>> example which want to use it, for AppArmor is currently abusing the cred >>> a little bit to store the change_hat and setexeccon info. Although >>> security_task_free() hook was revived in Linux 3.4 because Yama LSM module >>> wanted to release per "struct task_struct" security blob, >>> security_task_alloc() hook and "struct task_struct"->security field were >>> not revived. Nowadays, we are getting proposals of lightweight LSM modules >>> which want to use per "struct task_struct" security blob. PTAGS LSM module >>> and CaitSith LSM module which are currently under proposal for inclusion >>> also want to use it. Therefore, it will be time to revive >>> security_task_alloc() hook. >>> >>> We are already allowing multiple concurrent LSM modules (up to one fully >>> armored module which uses "struct cred"->security field or exclusive hooks >>> like security_xfrm_state_pol_flow_match(), plus unlimited number of >>> lightweight modules which do not use "struct cred"->security nor exclusive >>> hooks) as long as they are built into the kernel. Since multiple LSM >>> modules might want to use "struct task_struct"->security field, we need to >>> somehow calculate and allocate enough size for that field. Casey is trying >>> to calculate and allocate enough size for "struct cred"->security field, >>> but that approach is not applicable for LKM based LSM modules. On the other >>> hand, lightweight LSM modules (e.g. Yama) do not always allocate "struct >>> task_struct"->security field. If we tolerate managing per "struct >>> task_struct" security blobs outside of "struct task_struct", we don't need >>> to calculate and allocate enough size for "struct task_struct"->security >>> field, we can save memory by using address of "struct task_struct" as >>> a hash key for searching corresponding security blobs, and as a result >>> we can also allow LKM based LSM modules. Therefore, this patch does not >>> revive "struct task_struct"->security field. >> Hashing is an interesting approach for low bandwidth requests. >> >> I was more thinking on a struct within task like below: >> >> #ifdef CONFIG_SECURITY >> struct { >> # ifdef CONFIG_SECURITY_APPARMOR >> void *apparmor; >> # endif >> # ifdef CONFIG_SECURITY_PTAGS >> void *ptags; >> # endif >> } security; >> #endif >> >> It potential defines 2 "security" fields: security.apparmo and security.ptags > > This is not going to be popular with distributions like Ubuntu > that compile in all security modules. You're usually going to > have wasted space in the task structure doing this. > yes and no. Its a matter of picking your poison. - Bloat the task structs with all possible LSMs - add a security field to the task struct that points to a list or vector for the given security modules - list: flexible, doesn't waste memory but has extra lookup overhead - vector: quick lookup, but still has as much wasted space as embedding the whole set in the task hook I think adding the single field to the task struct has several advantages. - it is going to be more amenable to those maintaining the task struct - it doesn't require each new LSM module to change the task struct - provides a fixed known size of bloat - matches well with single LSM use case We are going to have overhead one way or another for distros like ubuntu. I just don't think the above approach is the best approach for the rest of the kernel community who don't want to deal with the LSM. My plan was to revive the single security field, go the major LSM claiming it route for the first iteration. The decision on how the infrastructure would manage and share it could then be pushed off to another patch set. This would allow us to separate the two discussions, and make the initial patch simpler and easier to get up. -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/04/2017 03:00 AM, Tetsuo Handa wrote: > ---------- > John Johansen wrote at http://lkml.kernel.org/r/c83a7281-367f-2459-3bcd-f15b64d950a6@canonical.com : >> On 12/20/2016 12:03 PM, Casey Schaufler wrote: >>> Whether or not José moves forward with PTAGS we have identified >>> an issue with the current cred based hooks for AppArmor, TOMOYO >>> and at least one other proposed module. Regardless of the issues >>> of /proc/pid there is work to be done. >>> >> I can take a stab at it in a few weeks. While not critical for apparmor >> it would certainly cleanup apparmor's cred handling. > > John might be still busy or offline now. So, here is my implementation. thanks Tetsuo, I hadn't actually started it yet. I've bee both busy and mostly offline for the last two weeks so you can image the mail backlog I'm saving the rest of my comments for the revised patch -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 1/6/2017 4:30 PM, John Johansen wrote: > On 01/04/2017 09:48 AM, Casey Schaufler wrote: >> On 1/4/2017 4:21 AM, José Bollo wrote: >>> Hi all, >>> >>> some remark below... >>> >>> Le mercredi 04 janvier 2017 à 20:00 +0900, Tetsuo Handa a écrit : >>>> ---------- >>>> John Johansen wrote at http://lkml.kernel.org/r/c83a7281-367f-2459-3bcd-f15b64d950a6@canonical.com : >>>>> On 12/20/2016 12:03 PM, Casey Schaufler wrote: >>>>>> Whether or not José moves forward with PTAGS we have identified >>>>>> an issue with the current cred based hooks for AppArmor, TOMOYO >>>>>> and at least one other proposed module. Regardless of the issues >>>>>> of /proc/pid there is work to be done. >>>>>> >>>>> I can take a stab at it in a few weeks. While not critical for apparmor >>>>> it would certainly cleanup apparmor's cred handling. >>>> John might be still busy or offline now. So, here is my implementation. >>>> ---------- >>>> >>>> We switched from "struct task_struct"->security to "struct cred"->security >>>> in Linux 2.6.29. But not all LSM modules were happy with that change. >>>> TOMOYO LSM module is an example which want to use per "struct task_struct" >>>> security blob, for TOMOYO's security context is defined based on "struct >>>> task_struct" rather than "struct cred". AppArmor LSM module is another >>>> example which want to use it, for AppArmor is currently abusing the cred >>>> a little bit to store the change_hat and setexeccon info. Although >>>> security_task_free() hook was revived in Linux 3.4 because Yama LSM module >>>> wanted to release per "struct task_struct" security blob, >>>> security_task_alloc() hook and "struct task_struct"->security field were >>>> not revived. Nowadays, we are getting proposals of lightweight LSM modules >>>> which want to use per "struct task_struct" security blob. PTAGS LSM module >>>> and CaitSith LSM module which are currently under proposal for inclusion >>>> also want to use it. Therefore, it will be time to revive >>>> security_task_alloc() hook. >>>> >>>> We are already allowing multiple concurrent LSM modules (up to one fully >>>> armored module which uses "struct cred"->security field or exclusive hooks >>>> like security_xfrm_state_pol_flow_match(), plus unlimited number of >>>> lightweight modules which do not use "struct cred"->security nor exclusive >>>> hooks) as long as they are built into the kernel. Since multiple LSM >>>> modules might want to use "struct task_struct"->security field, we need to >>>> somehow calculate and allocate enough size for that field. Casey is trying >>>> to calculate and allocate enough size for "struct cred"->security field, >>>> but that approach is not applicable for LKM based LSM modules. On the other >>>> hand, lightweight LSM modules (e.g. Yama) do not always allocate "struct >>>> task_struct"->security field. If we tolerate managing per "struct >>>> task_struct" security blobs outside of "struct task_struct", we don't need >>>> to calculate and allocate enough size for "struct task_struct"->security >>>> field, we can save memory by using address of "struct task_struct" as >>>> a hash key for searching corresponding security blobs, and as a result >>>> we can also allow LKM based LSM modules. Therefore, this patch does not >>>> revive "struct task_struct"->security field. >>> Hashing is an interesting approach for low bandwidth requests. >>> >>> I was more thinking on a struct within task like below: >>> >>> #ifdef CONFIG_SECURITY >>> struct { >>> # ifdef CONFIG_SECURITY_APPARMOR >>> void *apparmor; >>> # endif >>> # ifdef CONFIG_SECURITY_PTAGS >>> void *ptags; >>> # endif >>> } security; >>> #endif >>> >>> It potential defines 2 "security" fields: security.apparmo and security.ptags >> This is not going to be popular with distributions like Ubuntu >> that compile in all security modules. You're usually going to >> have wasted space in the task structure doing this. >> > yes and no. Its a matter of picking your poison. > - Bloat the task structs with all possible LSMs > - add a security field to the task struct that points to a list or vector for > the given security modules > - list: flexible, doesn't waste memory but has extra lookup overhead > - vector: quick lookup, but still has as much wasted space as embedding > the whole set in the task hook > > I think adding the single field to the task struct has several advantages. > - it is going to be more amenable to those maintaining the task struct > - it doesn't require each new LSM module to change the task struct > - provides a fixed known size of bloat > - matches well with single LSM use case > > We are going to have overhead one way or another for distros like ubuntu. > I just don't think the above approach is the best approach for the rest > of the kernel community who don't want to deal with the LSM. > > My plan was to revive the single security field, go the major LSM claiming > it route for the first iteration. > > The decision on how the infrastructure would manage and share it could > then be pushed off to another patch set. This would allow us to separate > the two discussions, and make the initial patch simpler and easier to > get up. I hope to have a revised extreme stacking patchset shortly after James updates security-next. I am using an infrastructure managed blob scheme, where the modules that are being used declare how much space they need. A task blob would be handled the same way as an inode, cred or other blob. It can handle blobs getting bigger during runtime with a touch of added overhead, although that won't be in this version. In other words, there's probably no need to spend too much effort on how to share a task blob as I expect we'll have a general shared blob solution "real soon". If my solution is unacceptable there are others available. > -- > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/06/2017 04:45 PM, Casey Schaufler wrote: > On 1/6/2017 4:30 PM, John Johansen wrote: >> On 01/04/2017 09:48 AM, Casey Schaufler wrote: >>> On 1/4/2017 4:21 AM, José Bollo wrote: >>>> Hi all, >>>> >>>> some remark below... >>>> >>>> Le mercredi 04 janvier 2017 à 20:00 +0900, Tetsuo Handa a écrit : >>>>> ---------- >>>>> John Johansen wrote at http://lkml.kernel.org/r/c83a7281-367f-2459-3bcd-f15b64d950a6@canonical.com : >>>>>> On 12/20/2016 12:03 PM, Casey Schaufler wrote: >>>>>>> Whether or not José moves forward with PTAGS we have identified >>>>>>> an issue with the current cred based hooks for AppArmor, TOMOYO >>>>>>> and at least one other proposed module. Regardless of the issues >>>>>>> of /proc/pid there is work to be done. >>>>>>> >>>>>> I can take a stab at it in a few weeks. While not critical for apparmor >>>>>> it would certainly cleanup apparmor's cred handling. >>>>> John might be still busy or offline now. So, here is my implementation. >>>>> ---------- >>>>> >>>>> We switched from "struct task_struct"->security to "struct cred"->security >>>>> in Linux 2.6.29. But not all LSM modules were happy with that change. >>>>> TOMOYO LSM module is an example which want to use per "struct task_struct" >>>>> security blob, for TOMOYO's security context is defined based on "struct >>>>> task_struct" rather than "struct cred". AppArmor LSM module is another >>>>> example which want to use it, for AppArmor is currently abusing the cred >>>>> a little bit to store the change_hat and setexeccon info. Although >>>>> security_task_free() hook was revived in Linux 3.4 because Yama LSM module >>>>> wanted to release per "struct task_struct" security blob, >>>>> security_task_alloc() hook and "struct task_struct"->security field were >>>>> not revived. Nowadays, we are getting proposals of lightweight LSM modules >>>>> which want to use per "struct task_struct" security blob. PTAGS LSM module >>>>> and CaitSith LSM module which are currently under proposal for inclusion >>>>> also want to use it. Therefore, it will be time to revive >>>>> security_task_alloc() hook. >>>>> >>>>> We are already allowing multiple concurrent LSM modules (up to one fully >>>>> armored module which uses "struct cred"->security field or exclusive hooks >>>>> like security_xfrm_state_pol_flow_match(), plus unlimited number of >>>>> lightweight modules which do not use "struct cred"->security nor exclusive >>>>> hooks) as long as they are built into the kernel. Since multiple LSM >>>>> modules might want to use "struct task_struct"->security field, we need to >>>>> somehow calculate and allocate enough size for that field. Casey is trying >>>>> to calculate and allocate enough size for "struct cred"->security field, >>>>> but that approach is not applicable for LKM based LSM modules. On the other >>>>> hand, lightweight LSM modules (e.g. Yama) do not always allocate "struct >>>>> task_struct"->security field. If we tolerate managing per "struct >>>>> task_struct" security blobs outside of "struct task_struct", we don't need >>>>> to calculate and allocate enough size for "struct task_struct"->security >>>>> field, we can save memory by using address of "struct task_struct" as >>>>> a hash key for searching corresponding security blobs, and as a result >>>>> we can also allow LKM based LSM modules. Therefore, this patch does not >>>>> revive "struct task_struct"->security field. >>>> Hashing is an interesting approach for low bandwidth requests. >>>> >>>> I was more thinking on a struct within task like below: >>>> >>>> #ifdef CONFIG_SECURITY >>>> struct { >>>> # ifdef CONFIG_SECURITY_APPARMOR >>>> void *apparmor; >>>> # endif >>>> # ifdef CONFIG_SECURITY_PTAGS >>>> void *ptags; >>>> # endif >>>> } security; >>>> #endif >>>> >>>> It potential defines 2 "security" fields: security.apparmo and security.ptags >>> This is not going to be popular with distributions like Ubuntu >>> that compile in all security modules. You're usually going to >>> have wasted space in the task structure doing this. >>> >> yes and no. Its a matter of picking your poison. >> - Bloat the task structs with all possible LSMs >> - add a security field to the task struct that points to a list or vector for >> the given security modules >> - list: flexible, doesn't waste memory but has extra lookup overhead >> - vector: quick lookup, but still has as much wasted space as embedding >> the whole set in the task hook >> >> I think adding the single field to the task struct has several advantages. >> - it is going to be more amenable to those maintaining the task struct >> - it doesn't require each new LSM module to change the task struct >> - provides a fixed known size of bloat >> - matches well with single LSM use case >> >> We are going to have overhead one way or another for distros like ubuntu. >> I just don't think the above approach is the best approach for the rest >> of the kernel community who don't want to deal with the LSM. >> >> My plan was to revive the single security field, go the major LSM claiming >> it route for the first iteration. >> >> The decision on how the infrastructure would manage and share it could >> then be pushed off to another patch set. This would allow us to separate >> the two discussions, and make the initial patch simpler and easier to >> get up. > > I hope to have a revised extreme stacking patchset > shortly after James updates security-next. I am using an > infrastructure managed blob scheme, where the modules that > are being used declare how much space they need. A task > blob would be handled the same way as an inode, cred or > other blob. It can handle blobs getting bigger during > runtime with a touch of added overhead, although that > won't be in this version. > > In other words, there's probably no need to spend too > much effort on how to share a task blob as I expect > we'll have a general shared blob solution "real soon". > If my solution is unacceptable there are others available. > perfect, all the more reason not to bother with a sharing solution here -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/04/2017 09:42 AM, Casey Schaufler wrote: > On 1/4/2017 3:00 AM, Tetsuo Handa wrote: >> ---------- >> John Johansen wrote at http://lkml.kernel.org/r/c83a7281-367f-2459-3bcd-f15b64d950a6@canonical.com : >>> On 12/20/2016 12:03 PM, Casey Schaufler wrote: << snip >> > > I would expect to have at least one module that uses > the revived hook included. > Well once we can settle on what this patch should look like, I am willing to provide and apparmor patch to use it and they can be proposed together. -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 0dde959..efa1354d 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -534,8 +534,18 @@ * manual page for definitions of the @clone_flags. * @clone_flags contains the flags indicating what should be shared. * Return 0 if permission is granted. + * @task_alloc: + * @task task being allocated. + * Handle allocation of task-related resources. Note that task_free is + * called even if task_alloc failed. This means that all task_free users + * have to be prepared for task_free being called without corresponding + * task_alloc call. Since the address of @task is guaranteed to remain + * unchanged between task_alloc call and task_free call, task_free users + * can use the address of @task for checking whether task_free is called + * without corresponding task_alloc call. + * Returns a zero on success, negative values on failure. * @task_free: - * @task task being freed + * @task task about to be freed. * Handle release of task-related resources. (Note that this can be called * from interrupt context.) * @cred_alloc_blank: @@ -1479,6 +1489,7 @@ int (*file_open)(struct file *file, const struct cred *cred); int (*task_create)(unsigned long clone_flags); + int (*task_alloc)(struct task_struct *task); void (*task_free)(struct task_struct *task); int (*cred_alloc_blank)(struct cred *cred, gfp_t gfp); void (*cred_free)(struct cred *cred); @@ -1744,6 +1755,7 @@ struct security_hook_heads { struct list_head file_receive; struct list_head file_open; struct list_head task_create; + struct list_head task_alloc; struct list_head task_free; struct list_head cred_alloc_blank; struct list_head cred_free; diff --git a/include/linux/security.h b/include/linux/security.h index f4ebac1..d7cb449 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -305,6 +305,7 @@ int security_file_send_sigiotask(struct task_struct *tsk, int security_file_receive(struct file *file); int security_file_open(struct file *file, const struct cred *cred); int security_task_create(unsigned long clone_flags); +int security_task_alloc(struct task_struct *task); void security_task_free(struct task_struct *task); int security_cred_alloc_blank(struct cred *cred, gfp_t gfp); void security_cred_free(struct cred *cred); @@ -857,6 +858,11 @@ static inline int security_task_create(unsigned long clone_flags) return 0; } +static inline int security_task_alloc(struct task_struct *task) +{ + return 0; +} + static inline void security_task_free(struct task_struct *task) { } diff --git a/kernel/fork.c b/kernel/fork.c index 11c5c8a..9094a48 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -1643,6 +1643,9 @@ static __latent_entropy struct task_struct *copy_process( goto bad_fork_cleanup_perf; /* copy all the process information */ shm_init_task(p); + retval = security_task_alloc(p); + if (retval) + goto bad_fork_cleanup_audit; retval = copy_semundo(clone_flags, p); if (retval) goto bad_fork_cleanup_audit; @@ -1862,6 +1865,7 @@ static __latent_entropy struct task_struct *copy_process( exit_sem(p); bad_fork_cleanup_audit: audit_free(p); + security_task_free(p); bad_fork_cleanup_perf: perf_event_free_task(p); bad_fork_cleanup_policy: diff --git a/security/security.c b/security/security.c index 32052f5..b746f85 100644 --- a/security/security.c +++ b/security/security.c @@ -892,6 +892,20 @@ int security_task_create(unsigned long clone_flags) return call_int_hook(task_create, 0, clone_flags); } +int security_task_alloc(struct task_struct *task) +{ + /* + * Since multiple LSMs might supply this call, we might end up partial + * allocation. Fortunately, we can unconditionally call + * security_task_free() if security_task_alloc() failed because Yama + * (the only user of task_free call as of revival of task_alloc call) + * is prepared for being called unconditionally and we can as well make + * new users of task_free call be prepared for being called + * unconditionally. + */ + return call_int_hook(task_alloc, 0, task); +} + void security_task_free(struct task_struct *task) { call_void_hook(task_free, task); @@ -1731,6 +1745,7 @@ struct security_hook_heads security_hook_heads = { .file_receive = LIST_HEAD_INIT(security_hook_heads.file_receive), .file_open = LIST_HEAD_INIT(security_hook_heads.file_open), .task_create = LIST_HEAD_INIT(security_hook_heads.task_create), + .task_alloc = LIST_HEAD_INIT(security_hook_heads.task_alloc), .task_free = LIST_HEAD_INIT(security_hook_heads.task_free), .cred_alloc_blank = LIST_HEAD_INIT(security_hook_heads.cred_alloc_blank),