| Message ID | 161377735153.87807.7492842242100187888.stgit@sifl (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | Split security_task_getsecid() into subj and obj variants | expand |
On 2/19/21 3:29 PM, Paul Moore wrote: > 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(). > > Signed-off-by: Paul Moore <paul@paul-moore.com> This looks good, though I have some questions around whether _subj use is correct in selinux_binder_transaction(). So off to learn more binder > --- > security/selinux/hooks.c | 85 +++++++++++++++++++++++++++------------------- > 1 file changed, 49 insertions(+), 36 deletions(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index f311541c4972e..1c53000d28e37 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; > > @@ -2034,11 +2047,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_obj(mgr), SECCLASS_BINDER, > BINDER__SET_CONTEXT_MGR, NULL); > } > > @@ -2046,8 +2056,8 @@ 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_subj(from); > + u32 tosid = task_sid_subj(to); > int rc; > > if (mysid != fromsid) { > @@ -2066,11 +2076,9 @@ static int selinux_binder_transaction(struct task_struct *from, > 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_subj(from), task_sid_obj(to), > + SECCLASS_BINDER, BINDER__TRANSFER, > NULL); > } > > @@ -2078,7 +2086,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_subj(to); > struct file_security_struct *fsec = selinux_file(file); > struct dentry *dentry = file->f_path.dentry; > struct inode_security_struct *isec; > @@ -2114,10 +2122,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, > @@ -2130,15 +2138,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); > } > > @@ -2263,7 +2271,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; > @@ -3916,7 +3924,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; > > @@ -4135,47 +4143,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); > } > > @@ -4206,7 +4219,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; > @@ -4215,21 +4228,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); > } > > @@ -4248,14 +4261,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); > @@ -6148,7 +6161,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); > @@ -7143,8 +7156,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), >
On 2021-02-19 18:29, Paul Moore wrote: > 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(). > > Signed-off-by: Paul Moore <paul@paul-moore.com> FWIW Reviewed-by: Richard Guy Briggs <rgb@redhat.com> > --- > security/selinux/hooks.c | 85 +++++++++++++++++++++++++++------------------- > 1 file changed, 49 insertions(+), 36 deletions(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index f311541c4972e..1c53000d28e37 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; > > @@ -2034,11 +2047,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_obj(mgr), SECCLASS_BINDER, > BINDER__SET_CONTEXT_MGR, NULL); > } > > @@ -2046,8 +2056,8 @@ 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_subj(from); > + u32 tosid = task_sid_subj(to); > int rc; > > if (mysid != fromsid) { > @@ -2066,11 +2076,9 @@ static int selinux_binder_transaction(struct task_struct *from, > 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_subj(from), task_sid_obj(to), > + SECCLASS_BINDER, BINDER__TRANSFER, > NULL); > } > > @@ -2078,7 +2086,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_subj(to); > struct file_security_struct *fsec = selinux_file(file); > struct dentry *dentry = file->f_path.dentry; > struct inode_security_struct *isec; > @@ -2114,10 +2122,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, > @@ -2130,15 +2138,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); > } > > @@ -2263,7 +2271,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; > @@ -3916,7 +3924,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; > > @@ -4135,47 +4143,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); > } > > @@ -4206,7 +4219,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; > @@ -4215,21 +4228,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); > } > > @@ -4248,14 +4261,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); > @@ -6148,7 +6161,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); > @@ -7143,8 +7156,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), > > -- > Linux-audit mailing list > Linux-audit@redhat.com > https://listman.redhat.com/mailman/listinfo/linux-audit - RGB -- Richard Guy Briggs <rgb@redhat.com> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635
On 2/19/21 3:29 PM, Paul Moore wrote: > 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(). > > Signed-off-by: Paul Moore <paul@paul-moore.com> I have a couple of questions below but the rest looks good > --- > security/selinux/hooks.c | 85 +++++++++++++++++++++++++++------------------- > 1 file changed, 49 insertions(+), 36 deletions(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index f311541c4972e..1c53000d28e37 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; > > @@ -2034,11 +2047,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_obj(mgr), SECCLASS_BINDER, > BINDER__SET_CONTEXT_MGR, NULL); > } > > @@ -2046,8 +2056,8 @@ 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_subj(from); fromsid potentially gets used as both the subject and the object the following permission checks. It makes sense to use the same cred for both checks but what I am not sure about yet is whether its actually safe to use the subject sid when the task isn't current. ie. I am still trying to determine if there is a race here between the transaction request and the permission check. > + u32 tosid = task_sid_subj(to); its not clear to me that using the subj for to is correct > int rc; > > if (mysid != fromsid) { > @@ -2066,11 +2076,9 @@ static int selinux_binder_transaction(struct task_struct *from, > 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_subj(from), task_sid_obj(to), > + SECCLASS_BINDER, BINDER__TRANSFER, > NULL); > } > > @@ -2078,7 +2086,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_subj(to); same question about safety here > struct file_security_struct *fsec = selinux_file(file); > struct dentry *dentry = file->f_path.dentry; > struct inode_security_struct *isec; > @@ -2114,10 +2122,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, > @@ -2130,15 +2138,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); > } > > @@ -2263,7 +2271,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; > @@ -3916,7 +3924,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; > > @@ -4135,47 +4143,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); > } > > @@ -4206,7 +4219,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; > @@ -4215,21 +4228,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); > } > > @@ -4248,14 +4261,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); > @@ -6148,7 +6161,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); > @@ -7143,8 +7156,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), >
On Tue, Mar 9, 2021 at 10:06 PM John Johansen <john.johansen@canonical.com> wrote: > On 2/19/21 3:29 PM, Paul Moore wrote: > > 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(). > > > > Signed-off-by: Paul Moore <paul@paul-moore.com> > > I have a couple of questions below but the rest looks good > > > --- > > security/selinux/hooks.c | 85 +++++++++++++++++++++++++++------------------- > > 1 file changed, 49 insertions(+), 36 deletions(-) > > > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > index f311541c4972e..1c53000d28e37 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; > > > > @@ -2034,11 +2047,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_obj(mgr), SECCLASS_BINDER, > > BINDER__SET_CONTEXT_MGR, NULL); > > } > > > > @@ -2046,8 +2056,8 @@ 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_subj(from); > > fromsid potentially gets used as both the subject and the object the following > permission checks. It makes sense to use the same cred for both checks but > what I am not sure about yet is whether its actually safe to use the subject > sid when the task isn't current. > > ie. I am still trying to determine if there is a race here between the transaction > request and the permission check. Okay, I see what you are concerned about now ... and unfortunately I'm not seeing a lot of precedence in the kernel for this type of usage either; the closest I can find is something like task_lock(), but that doesn't seem to cover the subjective creds. In fact, looking at override_creds(), there is nothing preventing a task from changing it's subjective creds at any point in time. Beyond the task_sid_subj() code here, looking back at patch 1 and the use of security_task_getsecid_subj() we look to be mostly safe (where safe means we are only inspecting the current task) with the exception of the binder code once again. There are some other exceptions but they are in the ptrace and audit code, both of which should be okay given the nature and calling context of the code. The problem really does seem to be just binder, and as I look at binder userspace example code, I'm starting to wonder if binder is setup properly to operate sanely in a situation where a process overrides its subject creds. It may be that we always need to use the objective/real creds with binder. Jeff, any binder insight here you can share with us? > > + u32 tosid = task_sid_subj(to); > its not clear to me that using the subj for to is correct Yes, I believe you are correct. Jeff, I know you looked at this code already, but I'm guessing you may have missed this (just as I did when I wrote it); are you okay with changing 'tosid' in selinux_binder_transaction() to the task's objective credentials? -- paul moore www.paul-moore.com
On Wed, Mar 10, 2021 at 11:32 PM Paul Moore <paul@paul-moore.com> wrote: > On Tue, Mar 9, 2021 at 10:06 PM John Johansen > <john.johansen@canonical.com> wrote: > > On 2/19/21 3:29 PM, Paul Moore wrote: ... > > > @@ -2034,11 +2047,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_obj(mgr), SECCLASS_BINDER, > > > BINDER__SET_CONTEXT_MGR, NULL); > > > } > > > > > > @@ -2046,8 +2056,8 @@ 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_subj(from); > > > > fromsid potentially gets used as both the subject and the object the following > > permission checks. It makes sense to use the same cred for both checks but > > what I am not sure about yet is whether its actually safe to use the subject > > sid when the task isn't current. > > > > ie. I am still trying to determine if there is a race here between the transaction > > request and the permission check. > > Okay, I see what you are concerned about now ... and unfortunately I'm > not seeing a lot of precedence in the kernel for this type of usage > either; the closest I can find is something like task_lock(), but that > doesn't seem to cover the subjective creds. In fact, looking at > override_creds(), there is nothing preventing a task from changing > it's subjective creds at any point in time. > > Beyond the task_sid_subj() code here, looking back at patch 1 and the > use of security_task_getsecid_subj() we look to be mostly safe (where > safe means we are only inspecting the current task) with the exception > of the binder code once again. There are some other exceptions but > they are in the ptrace and audit code, both of which should be okay > given the nature and calling context of the code. > > The problem really does seem to be just binder, and as I look at > binder userspace example code, I'm starting to wonder if binder is > setup properly to operate sanely in a situation where a process > overrides its subject creds. It may be that we always need to use the > objective/real creds with binder. Jeff, any binder insight here you > can share with us? > > > > + u32 tosid = task_sid_subj(to); > > its not clear to me that using the subj for to is correct > > Yes, I believe you are correct. Jeff, I know you looked at this code > already, but I'm guessing you may have missed this (just as I did when > I wrote it); are you okay with changing 'tosid' in > selinux_binder_transaction() to the task's objective credentials? Hearing no comments from the Android/binder folks, I'm in the process of switching this patchset to always use the objective creds in the case of binder. It's safe and I'm not sure binder is really prepared for the idea of a task changing it's creds anyway. Once the kernel builds and passes some basic sanity checks I'll repost the patches for review and inclusion, minus the AppArmor patch.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index f311541c4972e..1c53000d28e37 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; @@ -2034,11 +2047,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_obj(mgr), SECCLASS_BINDER, BINDER__SET_CONTEXT_MGR, NULL); } @@ -2046,8 +2056,8 @@ 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_subj(from); + u32 tosid = task_sid_subj(to); int rc; if (mysid != fromsid) { @@ -2066,11 +2076,9 @@ static int selinux_binder_transaction(struct task_struct *from, 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_subj(from), task_sid_obj(to), + SECCLASS_BINDER, BINDER__TRANSFER, NULL); } @@ -2078,7 +2086,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_subj(to); struct file_security_struct *fsec = selinux_file(file); struct dentry *dentry = file->f_path.dentry; struct inode_security_struct *isec; @@ -2114,10 +2122,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, @@ -2130,15 +2138,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); } @@ -2263,7 +2271,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; @@ -3916,7 +3924,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; @@ -4135,47 +4143,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); } @@ -4206,7 +4219,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; @@ -4215,21 +4228,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); } @@ -4248,14 +4261,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); @@ -6148,7 +6161,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); @@ -7143,8 +7156,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),
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(). Signed-off-by: Paul Moore <paul@paul-moore.com> --- security/selinux/hooks.c | 85 +++++++++++++++++++++++++++------------------- 1 file changed, 49 insertions(+), 36 deletions(-)