Of the three LSMs that implement the security_task_getsecid() LSM hook, all three LSMs provide the task's objective security credentials. This turns out to be unfortunate as most of the hook's callers seem to expect the task's subjective credentials, although a small handful of callers do correctly expect the objective credentials. This patch is the first step towards fixing the problem: it splits the existing security_task_getsecid() hook into two variants, one for the subjective creds, one for the objective creds. void security_task_getsecid_subj(struct task_struct *p, u32 *secid); void security_task_getsecid_obj(struct task_struct *p, u32 *secid); While this patch does fix all of the callers to use the correct variant, in order to keep this patch focused on the callers and to ease review, the LSMs continue to use the same implementation for both hooks. The net effect is that this patch should not change the behavior of the kernel in any way, it will be up to the latter LSM specific patches in this series to change the hook implementations and return the correct credentials. Acked-by: Mimi Zohar <zohar(a)linux.ibm.com&gt; (IMA) Signed-off-by: Paul Moore <paul(a)paul-moore.com&gt; --- drivers/android/binder.c | 11 ++++++++++- include/linux/cred.h | 2 +- include/linux/lsm_hook_defs.h | 5 ++++- include/linux/lsm_hooks.h | 12 +++++++++--- include/linux/security.h | 10 ++++++++-- kernel/audit.c | 4 ++-- kernel/auditfilter.c | 3 ++- kernel/auditsc.c | 8 ++++---- kernel/bpf/bpf_lsm.c | 3 ++- net/netlabel/netlabel_unlabeled.c | 2 +- net/netlabel/netlabel_user.h | 2 +- security/apparmor/lsm.c | 3 ++- security/integrity/ima/ima_appraise.c | 2 +- security/integrity/ima/ima_main.c | 14 +++++++------- security/security.c | 13 ++++++++++--- security/selinux/hooks.c | 3 ++- security/smack/smack_lsm.c | 3 ++- 17 files changed, 68 insertions(+), 32 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index c119736ca56a..61d235b6ccd8 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -2700,7 +2700,16 @@ static void binder_transaction(struct binder_proc *proc, u32 secid; size_t added_size; - security_task_getsecid(proc->tsk, &secid); + /* + * Arguably this should be the task's subjective LSM secid but + * we can't reliably access the subjective creds of a task + * other than our own so we must use the objective creds, which + * are safe to access. The downside is that if a task is + * temporarily overriding it's creds it will not be reflected + * here; however, it isn't clear that binder would handle that + * case well anyway. + */ + security_task_getsecid_obj(proc->tsk, &secid); ret = security_secid_to_secctx(secid, &secctx, &secctx_sz); if (ret) { return_error = BR_FAILED_REPLY; diff --git a/include/linux/cred.h b/include/linux/cred.h index 4c6350503697..ac0e5f97d7d8 100644 --- a/include/linux/cred.h +++ b/include/linux/cred.h @@ -140,7 +140,7 @@ struct cred { struct key *request_key_auth; /* assumed request_key authority */ #endif #ifdef CONFIG_SECURITY - void *security; /* subjective LSM security */ + void *security; /* LSM security */ #endif struct user_struct *user; /* real user ID subscription */ struct user_namespace *user_ns; /* user_ns the caps and keyrings are relative to. */ diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 477a597db013..3ad8085e85e1 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -203,7 +203,10 @@ LSM_HOOK(int, 0, task_fix_setgid, struct cred *new, const struct cred * old, LSM_HOOK(int, 0, task_setpgid, struct task_struct *p, pid_t pgid) LSM_HOOK(int, 0, task_getpgid, struct task_struct *p) LSM_HOOK(int, 0, task_getsid, struct task_struct *p) -LSM_HOOK(void, LSM_RET_VOID, task_getsecid, struct task_struct *p, u32 *secid) +LSM_HOOK(void, LSM_RET_VOID, task_getsecid_subj, + struct task_struct *p, u32 *secid) +LSM_HOOK(void, LSM_RET_VOID, task_getsecid_obj, + struct task_struct *p, u32 *secid) LSM_HOOK(int, 0, task_setnice, struct task_struct *p, int nice) LSM_HOOK(int, 0, task_setioprio, struct task_struct *p, int ioprio) LSM_HOOK(int, 0, task_getioprio, struct task_struct *p) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index fb7f3193753d..e25d31fe787e 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -707,9 +707,15 @@ * @p. * @p contains the task_struct for the process. * Return 0 if permission is granted. - * @task_getsecid: - * Retrieve the security identifier of the process @p. - * @p contains the task_struct for the process and place is into @secid. + * @task_getsecid_subj: + * Retrieve the subjective security identifier of the task_struct in @p + * and return it in @secid. Special care must be taken to ensure that @p + * is the either the "current" task, or the caller has exclusive access + * to @p. + * In case of failure, @secid will be set to zero. + * @task_getsecid_obj: + * Retrieve the objective security identifier of the task_struct in @p + * and return it in @secid. * In case of failure, @secid will be set to zero. * * @task_setnice: diff --git a/include/linux/security.h b/include/linux/security.h index 8aeebd6646dc..9c490bc437f5 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -414,7 +414,8 @@ int security_task_fix_setgid(struct cred *new, const struct cred *old, int security_task_setpgid(struct task_struct *p, pid_t pgid); int security_task_getpgid(struct task_struct *p); int security_task_getsid(struct task_struct *p); -void security_task_getsecid(struct task_struct *p, u32 *secid); +void security_task_getsecid_subj(struct task_struct *p, u32 *secid); +void security_task_getsecid_obj(struct task_struct *p, u32 *secid); int security_task_setnice(struct task_struct *p, int nice); int security_task_setioprio(struct task_struct *p, int ioprio); int security_task_getioprio(struct task_struct *p); @@ -1098,7 +1099,12 @@ static inline int security_task_getsid(struct task_struct *p) return 0; } -static inline void security_task_getsecid(struct task_struct *p, u32 *secid) +static inline void security_task_getsecid_subj(struct task_struct *p, u32 *secid) +{ + *secid = 0; +} + +static inline void security_task_getsecid_obj(struct task_struct *p, u32 *secid) { *secid = 0; } diff --git a/kernel/audit.c b/kernel/audit.c index 551a394bc8f4..121d37e700a6 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -2132,7 +2132,7 @@ int audit_log_task_context(struct audit_buffer *ab) int error; u32 sid; - security_task_getsecid(current, &sid); + security_task_getsecid_subj(current, &sid); if (!sid) return 0; @@ -2353,7 +2353,7 @@ int audit_signal_info(int sig, struct task_struct *t) audit_sig_uid = auid; else audit_sig_uid = uid; - security_task_getsecid(current, &audit_sig_sid); + security_task_getsecid_subj(current, &audit_sig_sid); } return audit_signal_info_syscall(t); diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c index 333b3bcfc545..db2c6b59dfc3 100644 --- a/kernel/auditfilter.c +++ b/kernel/auditfilter.c @@ -1359,7 +1359,8 @@ int audit_filter(int msgtype, unsigned int listtype) case AUDIT_SUBJ_SEN: case AUDIT_SUBJ_CLR: if (f->lsm_rule) { - security_task_getsecid(current, &sid); + security_task_getsecid_subj(current, + &sid); result = security_audit_rule_match(sid, f->type, f->op, f->lsm_rule); } diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 47fb48f42c93..9973865cbf13 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -667,7 +667,7 @@ static int audit_filter_rules(struct task_struct *tsk, logged upon error */ if (f->lsm_rule) { if (need_sid) { - security_task_getsecid(tsk, &sid); + security_task_getsecid_subj(tsk, &sid); need_sid = 0; } result = security_audit_rule_match(sid, f->type, @@ -2400,7 +2400,7 @@ void __audit_ptrace(struct task_struct *t) context->target_auid = audit_get_loginuid(t); context->target_uid = task_uid(t); context->target_sessionid = audit_get_sessionid(t); - security_task_getsecid(t, &context->target_sid); + security_task_getsecid_obj(t, &context->target_sid); memcpy(context->target_comm, t->comm, TASK_COMM_LEN); } @@ -2427,7 +2427,7 @@ int audit_signal_info_syscall(struct task_struct *t) ctx->target_auid = audit_get_loginuid(t); ctx->target_uid = t_uid; ctx->target_sessionid = audit_get_sessionid(t); - security_task_getsecid(t, &ctx->target_sid); + security_task_getsecid_obj(t, &ctx->target_sid); memcpy(ctx->target_comm, t->comm, TASK_COMM_LEN); return 0; } @@ -2448,7 +2448,7 @@ int audit_signal_info_syscall(struct task_struct *t) axp->target_auid[axp->pid_count] = audit_get_loginuid(t); axp->target_uid[axp->pid_count] = t_uid; axp->target_sessionid[axp->pid_count] = audit_get_sessionid(t); - security_task_getsecid(t, &axp->target_sid[axp->pid_count]); + security_task_getsecid_obj(t, &axp->target_sid[axp->pid_count]); memcpy(axp->target_comm[axp->pid_count], t->comm, TASK_COMM_LEN); axp->pid_count++; diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c index 1622a44d1617..0ff58259ccf8 100644 --- a/kernel/bpf/bpf_lsm.c +++ b/kernel/bpf/bpf_lsm.c @@ -209,7 +209,8 @@ BTF_ID(func, bpf_lsm_socket_socketpair) BTF_ID(func, bpf_lsm_syslog) BTF_ID(func, bpf_lsm_task_alloc) -BTF_ID(func, bpf_lsm_task_getsecid) +BTF_ID(func, bpf_lsm_task_getsecid_subj) +BTF_ID(func, bpf_lsm_task_getsecid_obj) BTF_ID(func, bpf_lsm_task_prctl) BTF_ID(func, bpf_lsm_task_setscheduler) BTF_ID(func, bpf_lsm_task_to_inode) diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c index ccb491642811..3e6ac9b790b1 100644 --- a/net/netlabel/netlabel_unlabeled.c +++ b/net/netlabel/netlabel_unlabeled.c @@ -1539,7 +1539,7 @@ int __init netlbl_unlabel_defconf(void) /* Only the kernel is allowed to call this function and the only time * it is called is at bootup before the audit subsystem is reporting * messages so don't worry to much about these values. */ - security_task_getsecid(current, &audit_info.secid); + security_task_getsecid_subj(current, &audit_info.secid); audit_info.loginuid = GLOBAL_ROOT_UID; audit_info.sessionid = 0; diff --git a/net/netlabel/netlabel_user.h b/net/netlabel/netlabel_user.h index 3c67afce64f1..b9ba8112b3c5 100644 --- a/net/netlabel/netlabel_user.h +++ b/net/netlabel/netlabel_user.h @@ -34,7 +34,7 @@ static inline void netlbl_netlink_auditinfo(struct sk_buff *skb, struct netlbl_audit *audit_info) { - security_task_getsecid(current, &audit_info->secid); + security_task_getsecid_subj(current, &audit_info->secid); audit_info->loginuid = audit_get_loginuid(current); audit_info->sessionid = audit_get_sessionid(current); } diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 240a53387e6b..f72406fe1bf2 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -1252,7 +1252,8 @@ static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(task_free, apparmor_task_free), LSM_HOOK_INIT(task_alloc, apparmor_task_alloc), - LSM_HOOK_INIT(task_getsecid, apparmor_task_getsecid), + LSM_HOOK_INIT(task_getsecid_subj, apparmor_task_getsecid), + LSM_HOOK_INIT(task_getsecid_obj, apparmor_task_getsecid), LSM_HOOK_INIT(task_setrlimit, apparmor_task_setrlimit), LSM_HOOK_INIT(task_kill, apparmor_task_kill), diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 565e33ff19d0..4e5eb0236278 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -76,7 +76,7 @@ int ima_must_appraise(struct user_namespace *mnt_userns, struct inode *inode, if (!ima_appraise) return 0; - security_task_getsecid(current, &secid); + security_task_getsecid_subj(current, &secid); return ima_match_policy(mnt_userns, inode, current_cred(), secid, func, mask, IMA_APPRAISE | IMA_HASH, NULL, NULL, NULL); } diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 9ef748ea829f..b85d9e429426 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -391,7 +391,7 @@ int ima_file_mmap(struct file *file, unsigned long prot) u32 secid; if (file && (prot & PROT_EXEC)) { - security_task_getsecid(current, &secid); + security_task_getsecid_subj(current, &secid); return process_measurement(file, current_cred(), secid, NULL, 0, MAY_EXEC, MMAP_CHECK); } @@ -429,7 +429,7 @@ int ima_file_mprotect(struct vm_area_struct *vma, unsigned long prot) !(prot & PROT_EXEC) || (vma->vm_flags & VM_EXEC)) return 0; - security_task_getsecid(current, &secid); + security_task_getsecid_subj(current, &secid); inode = file_inode(vma->vm_file); action = ima_get_action(file_mnt_user_ns(vma->vm_file), inode, current_cred(), secid, MAY_EXEC, MMAP_CHECK, @@ -470,7 +470,7 @@ int ima_bprm_check(struct linux_binprm *bprm) int ret; u32 secid; - security_task_getsecid(current, &secid); + security_task_getsecid_subj(current, &secid); ret = process_measurement(bprm->file, current_cred(), secid, NULL, 0, MAY_EXEC, BPRM_CHECK); if (ret) @@ -495,7 +495,7 @@ int ima_file_check(struct file *file, int mask) { u32 secid; - security_task_getsecid(current, &secid); + security_task_getsecid_subj(current, &secid); return process_measurement(file, current_cred(), secid, NULL, 0, mask & (MAY_READ | MAY_WRITE | MAY_EXEC | MAY_APPEND), FILE_CHECK); @@ -686,7 +686,7 @@ int ima_read_file(struct file *file, enum kernel_read_file_id read_id, /* Read entire file for all partial reads. */ func = read_idmap[read_id] ?: FILE_CHECK; - security_task_getsecid(current, &secid); + security_task_getsecid_subj(current, &secid); return process_measurement(file, current_cred(), secid, NULL, 0, MAY_READ, func); } @@ -729,7 +729,7 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size, } func = read_idmap[read_id] ?: FILE_CHECK; - security_task_getsecid(current, &secid); + security_task_getsecid_subj(current, &secid); return process_measurement(file, current_cred(), secid, buf, size, MAY_READ, func); } @@ -872,7 +872,7 @@ void process_buffer_measurement(struct user_namespace *mnt_userns, * buffer measurements. */ if (func) { - security_task_getsecid(current, &secid); + security_task_getsecid_subj(current, &secid); action = ima_get_action(mnt_userns, inode, current_cred(), secid, 0, func, &pcr, &template, func_data); diff --git a/security/security.c b/security/security.c index 5ac96b16f8fa..ee76d6c2f852 100644 --- a/security/security.c +++ b/security/security.c @@ -1762,12 +1762,19 @@ int security_task_getsid(struct task_struct *p) return call_int_hook(task_getsid, 0, p); } -void security_task_getsecid(struct task_struct *p, u32 *secid) +void security_task_getsecid_subj(struct task_struct *p, u32 *secid) { *secid = 0; - call_void_hook(task_getsecid, p, secid); + call_void_hook(task_getsecid_subj, p, secid); } -EXPORT_SYMBOL(security_task_getsecid); +EXPORT_SYMBOL(security_task_getsecid_subj); + +void security_task_getsecid_obj(struct task_struct *p, u32 *secid) +{ + *secid = 0; + call_void_hook(task_getsecid_obj, p, secid); +} +EXPORT_SYMBOL(security_task_getsecid_obj); int security_task_setnice(struct task_struct *p, int nice) { diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index eca9fc0ba764..24ca545c6e1b 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -7149,7 +7149,8 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(task_setpgid, selinux_task_setpgid), LSM_HOOK_INIT(task_getpgid, selinux_task_getpgid), LSM_HOOK_INIT(task_getsid, selinux_task_getsid), - LSM_HOOK_INIT(task_getsecid, selinux_task_getsecid), + LSM_HOOK_INIT(task_getsecid_subj, selinux_task_getsecid), + LSM_HOOK_INIT(task_getsecid_obj, selinux_task_getsecid), LSM_HOOK_INIT(task_setnice, selinux_task_setnice), LSM_HOOK_INIT(task_setioprio, selinux_task_setioprio), LSM_HOOK_INIT(task_getioprio, selinux_task_getioprio), diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 12a45e61c1a5..f546fb832f30 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -4759,7 +4759,8 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(task_setpgid, smack_task_setpgid), LSM_HOOK_INIT(task_getpgid, smack_task_getpgid), LSM_HOOK_INIT(task_getsid, smack_task_getsid), - LSM_HOOK_INIT(task_getsecid, smack_task_getsecid), + LSM_HOOK_INIT(task_getsecid_subj, smack_task_getsecid), + LSM_HOOK_INIT(task_getsecid_obj, smack_task_getsecid), LSM_HOOK_INIT(task_setnice, smack_task_setnice), LSM_HOOK_INIT(task_setioprio, smack_task_setioprio), LSM_HOOK_INIT(task_getioprio, smack_task_getioprio),

SELinux has a function, task_sid(), which returns the task's objective credentials, but unfortunately is used in a few places where the subjective task credentials should be used. Most notably in the new security_task_getsecid_subj() LSM hook. This patch fixes this and attempts to make things more obvious by introducing a new function, task_sid_subj(), and renaming the existing task_sid() function to task_sid_obj(). This patch also adds an interesting function in task_sid_binder(). The task_sid_binder() function has a comment which hopefully describes it's reason for being, but it basically boils down to the simple fact that we can't safely access another task's subjective credentials so in the case of binder we need to stick with the objective credentials regardless. Signed-off-by: Paul Moore <paul(a)paul-moore.com&gt; --- security/selinux/hooks.c | 112 ++++++++++++++++++++++++++++++---------------- 1 file changed, 73 insertions(+), 39 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 24ca545c6e1b..901a3ce453f1 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -229,10 +229,23 @@ static inline u32 cred_sid(const struct cred *cred) return tsec->sid; } +/* + * get the subjective security ID of a task + */ +static inline u32 task_sid_subj(const struct task_struct *task) +{ + u32 sid; + + rcu_read_lock(); + sid = cred_sid(rcu_dereference(task->cred)); + rcu_read_unlock(); + return sid; +} + /* * get the objective security ID of a task */ -static inline u32 task_sid(const struct task_struct *task) +static inline u32 task_sid_obj(const struct task_struct *task) { u32 sid; @@ -242,6 +255,29 @@ static inline u32 task_sid(const struct task_struct *task) return sid; } +/* + * get the security ID of a task for use with binder + */ +static inline u32 task_sid_binder(const struct task_struct *task) +{ + /* + * In many case where this function is used we should be using the + * task's subjective SID, but we can't reliably access the subjective + * creds of a task other than our own so we must use the objective + * creds/SID, which are safe to access. The downside is that if a task + * is temporarily overriding it's creds it will not be reflected here; + * however, it isn't clear that binder would handle that case well + * anyway. + * + * If this ever changes and we can safely reference the subjective + * creds/SID of another task, this function will make it easier to + * identify the various places where we make use of the task SIDs in + * the binder code. It is also likely that we will need to adjust + * the main drivers/android binder code as well. + */ + return task_sid_obj(task); +} + static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dentry); /* @@ -2035,11 +2071,8 @@ static inline u32 open_file_to_av(struct file *file) static int selinux_binder_set_context_mgr(struct task_struct *mgr) { - u32 mysid = current_sid(); - u32 mgrsid = task_sid(mgr); - return avc_has_perm(&selinux_state, - mysid, mgrsid, SECCLASS_BINDER, + current_sid(), task_sid_binder(mgr), SECCLASS_BINDER, BINDER__SET_CONTEXT_MGR, NULL); } @@ -2047,8 +2080,7 @@ static int selinux_binder_transaction(struct task_struct *from, struct task_struct *to) { u32 mysid = current_sid(); - u32 fromsid = task_sid(from); - u32 tosid = task_sid(to); + u32 fromsid = task_sid_binder(from); int rc; if (mysid != fromsid) { @@ -2059,19 +2091,16 @@ static int selinux_binder_transaction(struct task_struct *from, return rc; } - return avc_has_perm(&selinux_state, - fromsid, tosid, SECCLASS_BINDER, BINDER__CALL, - NULL); + return avc_has_perm(&selinux_state, fromsid, task_sid_binder(to), + SECCLASS_BINDER, BINDER__CALL, NULL); } static int selinux_binder_transfer_binder(struct task_struct *from, struct task_struct *to) { - u32 fromsid = task_sid(from); - u32 tosid = task_sid(to); - return avc_has_perm(&selinux_state, - fromsid, tosid, SECCLASS_BINDER, BINDER__TRANSFER, + task_sid_binder(from), task_sid_binder(to), + SECCLASS_BINDER, BINDER__TRANSFER, NULL); } @@ -2079,7 +2108,7 @@ static int selinux_binder_transfer_file(struct task_struct *from, struct task_struct *to, struct file *file) { - u32 sid = task_sid(to); + u32 sid = task_sid_binder(to); struct file_security_struct *fsec = selinux_file(file); struct dentry *dentry = file->f_path.dentry; struct inode_security_struct *isec; @@ -2115,10 +2144,10 @@ static int selinux_binder_transfer_file(struct task_struct *from, } static int selinux_ptrace_access_check(struct task_struct *child, - unsigned int mode) + unsigned int mode) { u32 sid = current_sid(); - u32 csid = task_sid(child); + u32 csid = task_sid_obj(child); if (mode & PTRACE_MODE_READ) return avc_has_perm(&selinux_state, @@ -2131,15 +2160,15 @@ static int selinux_ptrace_access_check(struct task_struct *child, static int selinux_ptrace_traceme(struct task_struct *parent) { return avc_has_perm(&selinux_state, - task_sid(parent), current_sid(), SECCLASS_PROCESS, - PROCESS__PTRACE, NULL); + task_sid_subj(parent), task_sid_obj(current), + SECCLASS_PROCESS, PROCESS__PTRACE, NULL); } static int selinux_capget(struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted) { return avc_has_perm(&selinux_state, - current_sid(), task_sid(target), SECCLASS_PROCESS, + current_sid(), task_sid_obj(target), SECCLASS_PROCESS, PROCESS__GETCAP, NULL); } @@ -2264,7 +2293,7 @@ static u32 ptrace_parent_sid(void) rcu_read_lock(); tracer = ptrace_parent(current); if (tracer) - sid = task_sid(tracer); + sid = task_sid_obj(tracer); rcu_read_unlock(); return sid; @@ -3921,7 +3950,7 @@ static int selinux_file_send_sigiotask(struct task_struct *tsk, struct fown_struct *fown, int signum) { struct file *file; - u32 sid = task_sid(tsk); + u32 sid = task_sid_obj(tsk); u32 perm; struct file_security_struct *fsec; @@ -4140,47 +4169,52 @@ static int selinux_kernel_load_data(enum kernel_load_data_id id, bool contents) static int selinux_task_setpgid(struct task_struct *p, pid_t pgid) { return avc_has_perm(&selinux_state, - current_sid(), task_sid(p), SECCLASS_PROCESS, + current_sid(), task_sid_obj(p), SECCLASS_PROCESS, PROCESS__SETPGID, NULL); } static int selinux_task_getpgid(struct task_struct *p) { return avc_has_perm(&selinux_state, - current_sid(), task_sid(p), SECCLASS_PROCESS, + current_sid(), task_sid_obj(p), SECCLASS_PROCESS, PROCESS__GETPGID, NULL); } static int selinux_task_getsid(struct task_struct *p) { return avc_has_perm(&selinux_state, - current_sid(), task_sid(p), SECCLASS_PROCESS, + current_sid(), task_sid_obj(p), SECCLASS_PROCESS, PROCESS__GETSESSION, NULL); } -static void selinux_task_getsecid(struct task_struct *p, u32 *secid) +static void selinux_task_getsecid_subj(struct task_struct *p, u32 *secid) +{ + *secid = task_sid_subj(p); +} + +static void selinux_task_getsecid_obj(struct task_struct *p, u32 *secid) { - *secid = task_sid(p); + *secid = task_sid_obj(p); } static int selinux_task_setnice(struct task_struct *p, int nice) { return avc_has_perm(&selinux_state, - current_sid(), task_sid(p), SECCLASS_PROCESS, + current_sid(), task_sid_obj(p), SECCLASS_PROCESS, PROCESS__SETSCHED, NULL); } static int selinux_task_setioprio(struct task_struct *p, int ioprio) { return avc_has_perm(&selinux_state, - current_sid(), task_sid(p), SECCLASS_PROCESS, + current_sid(), task_sid_obj(p), SECCLASS_PROCESS, PROCESS__SETSCHED, NULL); } static int selinux_task_getioprio(struct task_struct *p) { return avc_has_perm(&selinux_state, - current_sid(), task_sid(p), SECCLASS_PROCESS, + current_sid(), task_sid_obj(p), SECCLASS_PROCESS, PROCESS__GETSCHED, NULL); } @@ -4211,7 +4245,7 @@ static int selinux_task_setrlimit(struct task_struct *p, unsigned int resource, upon context transitions. See selinux_bprm_committing_creds. */ if (old_rlim->rlim_max != new_rlim->rlim_max) return avc_has_perm(&selinux_state, - current_sid(), task_sid(p), + current_sid(), task_sid_obj(p), SECCLASS_PROCESS, PROCESS__SETRLIMIT, NULL); return 0; @@ -4220,21 +4254,21 @@ static int selinux_task_setrlimit(struct task_struct *p, unsigned int resource, static int selinux_task_setscheduler(struct task_struct *p) { return avc_has_perm(&selinux_state, - current_sid(), task_sid(p), SECCLASS_PROCESS, + current_sid(), task_sid_obj(p), SECCLASS_PROCESS, PROCESS__SETSCHED, NULL); } static int selinux_task_getscheduler(struct task_struct *p) { return avc_has_perm(&selinux_state, - current_sid(), task_sid(p), SECCLASS_PROCESS, + current_sid(), task_sid_obj(p), SECCLASS_PROCESS, PROCESS__GETSCHED, NULL); } static int selinux_task_movememory(struct task_struct *p) { return avc_has_perm(&selinux_state, - current_sid(), task_sid(p), SECCLASS_PROCESS, + current_sid(), task_sid_obj(p), SECCLASS_PROCESS, PROCESS__SETSCHED, NULL); } @@ -4253,14 +4287,14 @@ static int selinux_task_kill(struct task_struct *p, struct kernel_siginfo *info, else secid = cred_sid(cred); return avc_has_perm(&selinux_state, - secid, task_sid(p), SECCLASS_PROCESS, perm, NULL); + secid, task_sid_obj(p), SECCLASS_PROCESS, perm, NULL); } static void selinux_task_to_inode(struct task_struct *p, struct inode *inode) { struct inode_security_struct *isec = selinux_inode(inode); - u32 sid = task_sid(p); + u32 sid = task_sid_obj(p); spin_lock(&isec->lock); isec->sclass = inode_mode_to_security_class(inode->i_mode); @@ -6153,7 +6187,7 @@ static int selinux_msg_queue_msgrcv(struct kern_ipc_perm *msq, struct msg_msg *m struct ipc_security_struct *isec; struct msg_security_struct *msec; struct common_audit_data ad; - u32 sid = task_sid(target); + u32 sid = task_sid_subj(target); int rc; isec = selinux_ipc(msq); @@ -7149,8 +7183,8 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(task_setpgid, selinux_task_setpgid), LSM_HOOK_INIT(task_getpgid, selinux_task_getpgid), LSM_HOOK_INIT(task_getsid, selinux_task_getsid), - LSM_HOOK_INIT(task_getsecid_subj, selinux_task_getsecid), - LSM_HOOK_INIT(task_getsecid_obj, selinux_task_getsecid), + LSM_HOOK_INIT(task_getsecid_subj, selinux_task_getsecid_subj), + LSM_HOOK_INIT(task_getsecid_obj, selinux_task_getsecid_obj), LSM_HOOK_INIT(task_setnice, selinux_task_setnice), LSM_HOOK_INIT(task_setioprio, selinux_task_setioprio), LSM_HOOK_INIT(task_getioprio, selinux_task_getioprio),

With the split of the security_task_getsecid() into subjective and objective variants it's time to update Smack to ensure it is using the correct task creds. Acked-by: Casey Schaufler <casey(a)schaufler-ca.com&gt; Reviewed-by: Richard Guy Briggs <rgb(a)redhat.com&gt; Reviewed-by: John Johansen <john.johansen(a)canonical.com&gt; Signed-off-by: Paul Moore <paul(a)paul-moore.com&gt; --- security/smack/smack.h | 18 +++++++++++++++++- security/smack/smack_lsm.c | 40 +++++++++++++++++++++++++++------------- 2 files changed, 44 insertions(+), 14 deletions(-) diff --git a/security/smack/smack.h b/security/smack/smack.h index a9768b12716b..08f9cb80655c 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -383,7 +383,23 @@ static inline struct smack_known *smk_of_task(const struct task_smack *tsp) return tsp->smk_task; } -static inline struct smack_known *smk_of_task_struct( +static inline struct smack_known *smk_of_task_struct_subj( + const struct task_struct *t) +{ + struct smack_known *skp; + const struct cred *cred; + + rcu_read_lock(); + + cred = rcu_dereference(t->cred); + skp = smk_of_task(smack_cred(cred)); + + rcu_read_unlock(); + + return skp; +} + +static inline struct smack_known *smk_of_task_struct_obj( const struct task_struct *t) { struct smack_known *skp; diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index f546fb832f30..cd14bec4ad80 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -159,7 +159,7 @@ static int smk_bu_current(char *note, struct smack_known *oskp, static int smk_bu_task(struct task_struct *otp, int mode, int rc) { struct task_smack *tsp = smack_cred(current_cred()); - struct smack_known *smk_task = smk_of_task_struct(otp); + struct smack_known *smk_task = smk_of_task_struct_obj(otp); char acc[SMK_NUM_ACCESS_TYPE + 1]; if (rc <= 0) @@ -479,7 +479,7 @@ static int smack_ptrace_access_check(struct task_struct *ctp, unsigned int mode) { struct smack_known *skp; - skp = smk_of_task_struct(ctp); + skp = smk_of_task_struct_obj(ctp); return smk_ptrace_rule_check(current, skp, mode, __func__); } @@ -2033,7 +2033,7 @@ static int smk_curacc_on_task(struct task_struct *p, int access, const char *caller) { struct smk_audit_info ad; - struct smack_known *skp = smk_of_task_struct(p); + struct smack_known *skp = smk_of_task_struct_subj(p); int rc; smk_ad_init(&ad, caller, LSM_AUDIT_DATA_TASK); @@ -2078,15 +2078,29 @@ static int smack_task_getsid(struct task_struct *p) } /** - * smack_task_getsecid - get the secid of the task - * @p: the object task + * smack_task_getsecid_subj - get the subjective secid of the task + * @p: the task * @secid: where to put the result * - * Sets the secid to contain a u32 version of the smack label. + * Sets the secid to contain a u32 version of the task's subjective smack label. + */ +static void smack_task_getsecid_subj(struct task_struct *p, u32 *secid) +{ + struct smack_known *skp = smk_of_task_struct_subj(p); + + *secid = skp->smk_secid; +} + +/** + * smack_task_getsecid_obj - get the objective secid of the task + * @p: the task + * @secid: where to put the result + * + * Sets the secid to contain a u32 version of the task's objective smack label. */ -static void smack_task_getsecid(struct task_struct *p, u32 *secid) +static void smack_task_getsecid_obj(struct task_struct *p, u32 *secid) { - struct smack_known *skp = smk_of_task_struct(p); + struct smack_known *skp = smk_of_task_struct_obj(p); *secid = skp->smk_secid; } @@ -2174,7 +2188,7 @@ static int smack_task_kill(struct task_struct *p, struct kernel_siginfo *info, { struct smk_audit_info ad; struct smack_known *skp; - struct smack_known *tkp = smk_of_task_struct(p); + struct smack_known *tkp = smk_of_task_struct_obj(p); int rc; if (!sig) @@ -2212,7 +2226,7 @@ static int smack_task_kill(struct task_struct *p, struct kernel_siginfo *info, static void smack_task_to_inode(struct task_struct *p, struct inode *inode) { struct inode_smack *isp = smack_inode(inode); - struct smack_known *skp = smk_of_task_struct(p); + struct smack_known *skp = smk_of_task_struct_obj(p); isp->smk_inode = skp; isp->smk_flags |= SMK_INODE_INSTANT; @@ -3483,7 +3497,7 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode) */ static int smack_getprocattr(struct task_struct *p, char *name, char **value) { - struct smack_known *skp = smk_of_task_struct(p); + struct smack_known *skp = smk_of_task_struct_subj(p); char *cp; int slen; @@ -4759,8 +4773,8 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(task_setpgid, smack_task_setpgid), LSM_HOOK_INIT(task_getpgid, smack_task_getpgid), LSM_HOOK_INIT(task_getsid, smack_task_getsid), - LSM_HOOK_INIT(task_getsecid_subj, smack_task_getsecid), - LSM_HOOK_INIT(task_getsecid_obj, smack_task_getsecid), + LSM_HOOK_INIT(task_getsecid_subj, smack_task_getsecid_subj), + LSM_HOOK_INIT(task_getsecid_obj, smack_task_getsecid_obj), LSM_HOOK_INIT(task_setnice, smack_task_setnice), LSM_HOOK_INIT(task_setioprio, smack_task_setioprio), LSM_HOOK_INIT(task_getioprio, smack_task_getioprio),