Subject: [07/38] LSM: Revive security_task_alloc()/security_task_free()/security_bprm_free() hooks. The "void *security" variable which is used by LSM modules is embedded into "struct cred". Thus, if current thread is doing do_execve(), current thread cannot access the "void *security" of the current thread's new "struct cred" (which is stored into "struct linux_binprm"->cred) from security_dentry_open(). This is conflicting with TOMOYO's permission checks for binary loader programs (e.g. /lib/ld-linux.so.2 ). TOMOYO wants to do permission checks for binary loader programs using "struct linux_binprm"->cred->security (rather than current->cred->security), but "struct linux_binprm" is not passed to security_dentry_open(). We are not permitted to call commit_creds() before the point of no return. Thus, calling commit_creds() before do_execve() succeeds cannot be a solution. There are two approaches for making "struct linux_binprm"->cred->security accessible from security_dentry_open(). One is to pass "struct linux_binprm" to security_dentry_open() and its callers. The other is to use cred override mechanisms, as described below. If we call override_creds()/revert_creds() like I proposed in "Re: [PATCH 0/9] Open loaders and interpreters with new creds during exec" ( http://www.spinics.net/linux/fedora/linux-security-module/msg10460.html ), current DAC behavior will change as David mentioned in "[PATCH 9/9] LSM: Use derived creds for accessing an executable's interpreter and binary loader" ( http://www.spinics.net/linux/fedora/linux-security-module/msg10457.html ). If we don't call override_creds()/revert_creds() until the point of no return, TOMOYO has to add bprm->cred to the list before calling search_binary_handler() (using security_bprm_set_creds() hook or security_bprm_check() hook) and remove bprm->cred from the list after returning from search_binary_handler() (using security_bprm_free() hook or security_cred_free() hook). Although, it is technically possible to remove bprm->cred from the list (without security_bprm_free() hook) by holding a reference to "struct linux_binprm"->cred and periodically checking whether the reference count became 1 or not, it is not clean. Therefore, to be able to cleanly remove bprm->cred from the list, this patch revives security_bprm_free() hook. Reviving security_bprm_free() hook helps TOMOYO with simplifying three more things in addition to solving the problem described above. First is that TOMOYO can use this hook for calling tomoyo_read_unlock(). Since this hook has been missing since 2.6.29, TOMOYO was not able to enclose the entire do_execve() operation within an SRCU section, forcing TOMOYO to manage refcounter for "struct tomoyo_domain_info" in order to close the race window between "time to find domain" and "time to transit to that domain". Second is that TOMOYO can use this hook for calling kfree(). In the subsequent patches, I will add a sort of redirector which will look like load_script() in fs/binfmt_script.c . Regarding load_script(), it can use stack memory for keeping the interpreter's pathname because it calls search_binary_handler() from it. But regarding TOMOYO, I will have to use kmalloc()/kfree() for keeping the handler's pathname because I think LSM modules are expected not to call search_binary_handler() >from themselves. Third is that TOMOYO's interactive enforcing can get up-to-date information because this hook is called as soon as do_execve() finishes (rather than after an RCU grace period). However, reviving security_bprm_free() hook alone is not sufficient for TOMOYO. As long as "struct cred" combines DAC attributes and MAC attributes, TOMOYO cannot work as expected. The credentials mechanism also conflicts with TOMOYO outside do_execve(), for the mapping of "struct cred" and "struct task_struct" is not always one to one. The definition of TOMOYO's domain is determined by only current thread's "struct task_struct" and the name of programs the current thread ever passed to do_execve() requests. The initial domain is "<kernel>" domain, which the kernel thread belongs to. The domain for /sbin/init which is executed by the kernel is "<kernel> /sbin/init". When the kernel thread executes /sbin/init , TOMOYO checks execute permission for /sbin/init using "<kernel>" domain and read permission for /lib/ld-linux.so.2 using "<kernel> /sbin/init" domain. This is because I consider it is not the fault of the kernel thread that /sbin/init cannot be executed without /lib/ld-linux.so.2 . I consider it is the fault of /sbin/init that /sbin/init cannot be executed without /lib/ld-linux.so.2 . When /bin/cat opens some file for reading, TOMOYO checks read permission of that file. If that file is on a normal filesystem (I mean filesystems where LSM hooks are not called after once security_dentry_open() is called with the pathname /bin/cat has requested), it is no problem. If that file is on a special filesystem (I mean filesystems where LSM hooks are called after once security_dentry_open() is called with the pathname /bin/cat has requested, such as stackable filesystems, networked filesystems, cachefiles), it is a bug for TOMOYO to check permissions using the domain for /bin/cat . It is not the fault of /bin/cat that /bin/cat needs both "read permission for that file" and "other permissions" (e.g. permission for reading other pathnames which /bin/cat has not requested, permission for sending/receiving UDP datagram). It is the fault of the process (e.g. the kernel thread) that made /bin/cat to require "other permissions". The definition of TOMOYO's policy is determined by only what current thread has requested. The in-kernel businesses must not ask the userspace for "other permissions". (Unfortunately, it is currently impossible because there is no hint that tells how the LSM hook is called (i.e. by current thread's request or by in-kernel businesses). As a result, unless in-kernel businesses are handled by a separated kernel thread, TOMOYO cannot help asking the current thread for "other permissions" when the LSM hook is called by in-kernel businesses. TOMOYO can work as expected only if TOMOYO's domains are managed using per "struct task_struct" variables and a hint for telling how the LSM hook is called is given.) The credential override mechanism is used for handling in-kernel businesses. But use of credential override mechanism violates the TOMOYO's policy definition because TOMOYO's policy must not contain "other permissions" required by in-kernel businesses. Cachefiles is an example where current TOMOYO code (i.e. using current cred rather than the cred passed to security_dentry_open() hook) works as expected. Use of credential for /bin/cat (in order to check permissions for /var/fscache/cache/@4a/I03nfs/@30/foo/bar on behalf of /bin/cat process) violates TOMOYO's policy. I consider that it is not the fault of /bin/cat that /bin/cat needs permission for /var/fscache/cache/@4a/I03nfs/@30/foo/bar in addition to the pathname /bin/cat has requested when reading a file cached by cachefiles. It is the fault of cachefiles if cachefiles asks /bin/cat process for permission for /var/fscache/cache/@4a/I03nfs/@30/foo/bar . TOMOYO wants to use per a "struct task_struct" variables but a "struct cred" might be shared between multiple threads. Also, a "struct task_struct" can temporarily have multiple "struct cred" instances but these instances are switched without calling LSM hooks. Therefore, TOMOYO cannot carry changes made against previous "struct cred"->security to next "struct cred"->security. These limitations indicate that "struct cred"->security is not suitable for maintaining per a "struct task_struct" variables. Although, it is technically possible to copy per a "struct task_struct" variables (without reviving security_task_alloc() hook) by associating with an external buffer using current->cred as a search key, it is not clean. Although, it is technically possible to release per a "struct task_struct" variables (without security_task_free() hook) by holding a reference to "struct task_struct" and periodically checking whether the reference count became 1 or not, it is not clean. Therefore, to be able to cleanly copy and release per a "struct task_struct" variables, this patch also revives security_task_alloc()/security_task_free() hooks. By reviving these hooks, TOMOYO can use per a "struct task_struct" variables (rather than per a "struct cred" variables) by maintaining "void *security" in an external hashtable (rather than "struct cred"), and TOMOYO will be able to work as expected. This patch will pave the way for running multiple LSM modules in parallel, for each LSM module can maintain "void *security" in its own table rather than scrambling for "void *security" of "struct cred" (e.g. SELinux/SMACK/AppArmor can use "struct cred"->security if TOMOYO uses external hashtable). Currently, TOMOYO is the only in-tree LSM module who wants this patch. But other currently out-of-tree LSM modules (e.g. YAMA) can use these hooks when it became possible to run multiple LSM modules in parallel. Signed-off-by: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx> --- fs/exec.c | 2 ++ include/linux/security.h | 31 +++++++++++++++++++++++++++++++ kernel/fork.c | 5 +++++ security/capability.c | 16 ++++++++++++++++ security/security.c | 15 +++++++++++++++ 5 files changed, 69 insertions(+) --- security-testing-2.6.orig/fs/exec.c +++ security-testing-2.6/fs/exec.c @@ -1143,7 +1143,9 @@ void free_bprm(struct linux_binprm *bprm if (bprm->cred) { mutex_unlock(¤t->signal->cred_guard_mutex); abort_creds(bprm->cred); + bprm->cred = NULL; } + security_bprm_free(bprm); kfree(bprm); } --- security-testing-2.6.orig/include/linux/security.h +++ security-testing-2.6/include/linux/security.h @@ -229,6 +229,9 @@ static inline void security_free_mnt_opt * on the initial stack to the ELF interpreter to indicate whether libc * should enable secure mode. * @bprm contains the linux_binprm structure. + * @bprm_free_security: + * Tell current thread that do_execve() has finished. + * @bprm contains the linux_binprm structure. * * Security hooks for filesystem operations. * @@ -645,6 +648,15 @@ static inline void security_free_mnt_opt * 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_security: + * Copy current thread's per task_struct variables to a chlid thread. + * Note that the task_struct does not provide ->security field. + * @p contains the task_struct for child process. + * Return 0 on success, negative value otherwise. + * @task_free_security: + * Clean up per task_struct variables. + * Note that this hook is also called when security_task_alloc() failed. + * @p contains the task_struct. * @cred_alloc_blank: * @cred points to the credentials. * @gfp indicates the atomicity of any memory allocations. @@ -1398,6 +1410,7 @@ struct security_operations { int (*bprm_secureexec) (struct linux_binprm *bprm); void (*bprm_committing_creds) (struct linux_binprm *bprm); void (*bprm_committed_creds) (struct linux_binprm *bprm); + void (*bprm_free_security) (struct linux_binprm *bprm); int (*sb_alloc_security) (struct super_block *sb); void (*sb_free_security) (struct super_block *sb); @@ -1495,6 +1508,8 @@ struct security_operations { int (*dentry_open) (struct file *file, const struct cred *cred); int (*task_create) (unsigned long clone_flags); + int (*task_alloc_security) (struct task_struct *p); + void (*task_free_security) (struct task_struct *p); int (*cred_alloc_blank) (struct cred *cred, gfp_t gfp); void (*cred_free) (struct cred *cred); int (*cred_prepare)(struct cred *new, const struct cred *old, @@ -1685,6 +1700,7 @@ int security_bprm_check(struct linux_bin void security_bprm_committing_creds(struct linux_binprm *bprm); void security_bprm_committed_creds(struct linux_binprm *bprm); int security_bprm_secureexec(struct linux_binprm *bprm); +void security_bprm_free(struct linux_binprm *bprm); int security_sb_alloc(struct super_block *sb); void security_sb_free(struct super_block *sb); int security_sb_copy_data(char *orig, char *copy); @@ -1753,6 +1769,8 @@ int security_file_send_sigiotask(struct int security_file_receive(struct file *file); int security_dentry_open(struct file *file, const struct cred *cred); int security_task_create(unsigned long clone_flags); +int security_task_alloc(struct task_struct *p); +void security_task_free(struct task_struct *p); int security_cred_alloc_blank(struct cred *cred, gfp_t gfp); void security_cred_free(struct cred *cred); int security_prepare_creds(struct cred *new, const struct cred *old, gfp_t gfp); @@ -1957,6 +1975,10 @@ static inline int security_bprm_secureex return cap_bprm_secureexec(bprm); } +static inline void security_bprm_free(struct linux_binprm *bprm) +{ +} + static inline int security_sb_alloc(struct super_block *sb) { return 0; @@ -2257,6 +2279,15 @@ static inline int security_dentry_open(s return 0; } +static inline int security_task_alloc(struct task_struct *p) +{ + return 0; +} + +static inline void security_task_free(struct task_struct *p) +{ +} + static inline int security_task_create(unsigned long clone_flags) { return 0; --- security-testing-2.6.orig/kernel/fork.c +++ security-testing-2.6/kernel/fork.c @@ -196,6 +196,7 @@ void __put_task_struct(struct task_struc delayacct_tsk_free(tsk); put_signal_struct(tsk->signal); + security_task_free(tsk); if (!profile_handoff_task(tsk)) free_task(tsk); } @@ -1161,6 +1162,9 @@ static struct task_struct *copy_process( if ((retval = audit_alloc(p))) goto bad_fork_cleanup_policy; + retval = security_task_alloc(p); + if (retval) + goto bad_fork_cleanup_audit; /* copy all the process information */ if ((retval = copy_semundo(clone_flags, p))) goto bad_fork_cleanup_audit; @@ -1345,6 +1349,7 @@ bad_fork_cleanup_semundo: exit_sem(p); bad_fork_cleanup_audit: audit_free(p); + security_task_free(p); bad_fork_cleanup_policy: perf_event_free_task(p); #ifdef CONFIG_NUMA --- security-testing-2.6.orig/security/capability.c +++ security-testing-2.6/security/capability.c @@ -40,6 +40,10 @@ static void cap_bprm_committed_creds(str { } +static void cap_bprm_free_security(struct linux_binprm *bprm) +{ +} + static int cap_sb_alloc_security(struct super_block *sb) { return 0; @@ -359,6 +363,15 @@ static int cap_task_create(unsigned long return 0; } +static int cap_task_alloc_security(struct task_struct *p) +{ + return 0; +} + +static void cap_task_free_security(struct task_struct *p) +{ +} + static int cap_cred_alloc_blank(struct cred *cred, gfp_t gfp) { return 0; @@ -889,6 +902,7 @@ void __init security_fixup_ops(struct se set_to_cap_if_null(ops, bprm_committed_creds); set_to_cap_if_null(ops, bprm_check_security); set_to_cap_if_null(ops, bprm_secureexec); + set_to_cap_if_null(ops, bprm_free_security); set_to_cap_if_null(ops, sb_alloc_security); set_to_cap_if_null(ops, sb_free_security); set_to_cap_if_null(ops, sb_copy_data); @@ -955,6 +969,8 @@ void __init security_fixup_ops(struct se set_to_cap_if_null(ops, file_receive); set_to_cap_if_null(ops, dentry_open); set_to_cap_if_null(ops, task_create); + set_to_cap_if_null(ops, task_alloc_security); + set_to_cap_if_null(ops, task_free_security); set_to_cap_if_null(ops, cred_alloc_blank); set_to_cap_if_null(ops, cred_free); set_to_cap_if_null(ops, cred_prepare); --- security-testing-2.6.orig/security/security.c +++ security-testing-2.6/security/security.c @@ -254,6 +254,11 @@ int security_bprm_secureexec(struct linu return security_ops->bprm_secureexec(bprm); } +void security_bprm_free(struct linux_binprm *bprm) +{ + security_ops->bprm_free_security(bprm); +} + int security_sb_alloc(struct super_block *sb) { return security_ops->sb_alloc_security(sb); @@ -704,6 +709,16 @@ int security_task_create(unsigned long c return security_ops->task_create(clone_flags); } +int security_task_alloc(struct task_struct *p) +{ + return security_ops->task_alloc_security(p); +} + +void security_task_free(struct task_struct *p) +{ + security_ops->task_free_security(p); +} + int security_cred_alloc_blank(struct cred *cred, gfp_t gfp) { return security_ops->cred_alloc_blank(cred, gfp); -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
