| Message ID | 20230315224704.2672-11-casey@schaufler-ca.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | LSM: Three basic syscalls | expand |
On Wed, Mar 15, 2023 at 6:52 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > > Add hooks for setselfattr and getselfattr. These hooks are not very > different from their setprocattr and getprocattr equivalents, and > much of the code is shared. > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > Cc: selinux@vger.kernel.org > Cc: Paul Moore <paul@paul-moore.com> > --- > security/selinux/hooks.c | 147 +++++++++++++++++++++++++++++++-------- > 1 file changed, 117 insertions(+), 30 deletions(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 9403aee75981..8896edf80aa9 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -6348,8 +6348,7 @@ static void selinux_d_instantiate(struct dentry *dentry, struct inode *inode) > inode_doinit_with_dentry(inode, dentry); > } > > -static int selinux_getprocattr(struct task_struct *p, > - const char *name, char **value) > +static int do_getattr(unsigned int attr, struct task_struct *p, char **value) Are you ready for more naming nitpicks? ;) Let's call this 'selinux_lsm_getattr()', and the matching setter should be 'selinux_lsm_setattr()'. > { > const struct task_security_struct *__tsec; > u32 sid; > @@ -6367,20 +6366,27 @@ static int selinux_getprocattr(struct task_struct *p, > goto bad; > } > > - if (!strcmp(name, "current")) > + switch (attr) { > + case LSM_ATTR_CURRENT: > sid = __tsec->sid; > - else if (!strcmp(name, "prev")) > + break; > + case LSM_ATTR_PREV: > sid = __tsec->osid; > - else if (!strcmp(name, "exec")) > + break; > + case LSM_ATTR_EXEC: > sid = __tsec->exec_sid; > - else if (!strcmp(name, "fscreate")) > + break; > + case LSM_ATTR_FSCREATE: > sid = __tsec->create_sid; > - else if (!strcmp(name, "keycreate")) > + break; > + case LSM_ATTR_KEYCREATE: > sid = __tsec->keycreate_sid; > - else if (!strcmp(name, "sockcreate")) > + break; > + case LSM_ATTR_SOCKCREATE: > sid = __tsec->sockcreate_sid; > - else { > - error = -EINVAL; > + break; > + default: > + error = -EOPNOTSUPP; The error should probably be -EINVAL. > goto bad; > } > rcu_read_unlock(); > @@ -6398,7 +6404,7 @@ static int selinux_getprocattr(struct task_struct *p, > return error; > } > > -static int selinux_setprocattr(const char *name, void *value, size_t size) > +static int do_setattr(u64 attr, void *value, size_t size) > { > struct task_security_struct *tsec; > struct cred *new; > @@ -6409,28 +6415,36 @@ static int selinux_setprocattr(const char *name, void *value, size_t size) > /* > * Basic control over ability to set these attributes at all. > */ > - if (!strcmp(name, "exec")) > + switch (attr) { > + case LSM_ATTR_CURRENT: > + error = avc_has_perm(&selinux_state, > + mysid, mysid, SECCLASS_PROCESS, > + PROCESS__SETCURRENT, NULL); > + break; > + case LSM_ATTR_EXEC: > error = avc_has_perm(&selinux_state, > mysid, mysid, SECCLASS_PROCESS, > PROCESS__SETEXEC, NULL); > - else if (!strcmp(name, "fscreate")) > + break; > + case LSM_ATTR_FSCREATE: > error = avc_has_perm(&selinux_state, > mysid, mysid, SECCLASS_PROCESS, > PROCESS__SETFSCREATE, NULL); > - else if (!strcmp(name, "keycreate")) > + break; > + case LSM_ATTR_KEYCREATE: > error = avc_has_perm(&selinux_state, > mysid, mysid, SECCLASS_PROCESS, > PROCESS__SETKEYCREATE, NULL); > - else if (!strcmp(name, "sockcreate")) > + break; > + case LSM_ATTR_SOCKCREATE: > error = avc_has_perm(&selinux_state, > mysid, mysid, SECCLASS_PROCESS, > PROCESS__SETSOCKCREATE, NULL); > - else if (!strcmp(name, "current")) > - error = avc_has_perm(&selinux_state, > - mysid, mysid, SECCLASS_PROCESS, > - PROCESS__SETCURRENT, NULL); > - else > - error = -EINVAL; > + break; > + default: > + error = -EOPNOTSUPP; Same as above, should be -EINVAL. > + break; > + } > if (error) > return error; > > @@ -6442,13 +6456,14 @@ static int selinux_setprocattr(const char *name, void *value, size_t size) > } > error = security_context_to_sid(&selinux_state, value, size, > &sid, GFP_KERNEL); > - if (error == -EINVAL && !strcmp(name, "fscreate")) { > + if (error == -EINVAL && attr == LSM_ATTR_FSCREATE) { > if (!has_cap_mac_admin(true)) { > struct audit_buffer *ab; > size_t audit_size; > > - /* We strip a nul only if it is at the end, otherwise the > - * context contains a nul and we should audit that */ > + /* We strip a nul only if it is at the end, > + * otherwise the context contains a nul and > + * we should audit that */ You *do* get gold stars for fixing line lengths in close proximity ;) > if (str[size - 1] == '\0') > audit_size = size - 1; > else > @@ -6459,7 +6474,8 @@ static int selinux_setprocattr(const char *name, void *value, size_t size) > if (!ab) > return error; > audit_log_format(ab, "op=fscreate invalid_context="); > - audit_log_n_untrustedstring(ab, value, audit_size); > + audit_log_n_untrustedstring(ab, value, > + audit_size); > audit_log_end(ab); > > return error; > @@ -6483,11 +6499,11 @@ static int selinux_setprocattr(const char *name, void *value, size_t size) > checks and may_create for the file creation checks. The > operation will then fail if the context is not permitted. */ > tsec = selinux_cred(new); > - if (!strcmp(name, "exec")) { > + if (attr == LSM_ATTR_EXEC) { > tsec->exec_sid = sid; > - } else if (!strcmp(name, "fscreate")) { > + } else if (attr == LSM_ATTR_FSCREATE) { > tsec->create_sid = sid; > - } else if (!strcmp(name, "keycreate")) { > + } else if (attr == LSM_ATTR_KEYCREATE) { > if (sid) { > error = avc_has_perm(&selinux_state, mysid, sid, > SECCLASS_KEY, KEY__CREATE, NULL); > @@ -6495,9 +6511,9 @@ static int selinux_setprocattr(const char *name, void *value, size_t size) > goto abort_change; > } > tsec->keycreate_sid = sid; > - } else if (!strcmp(name, "sockcreate")) { > + } else if (attr == LSM_ATTR_SOCKCREATE) { > tsec->sockcreate_sid = sid; > - } else if (!strcmp(name, "current")) { > + } else if (attr == LSM_ATTR_CURRENT) { > error = -EINVAL; > if (sid == 0) > goto abort_change; > @@ -6542,6 +6558,75 @@ static int selinux_setprocattr(const char *name, void *value, size_t size) > return error; > } > > +static int selinux_getselfattr(unsigned int __user attr, > + struct lsm_ctx __user *ctx, size_t *size, > + u32 __user flags) > +{ > + char *value; > + size_t total_len; > + int len; > + int rc = 0; > + > + len = do_getattr(attr, current, &value); > + if (len < 0) > + return len; > + > + total_len = len + sizeof(*ctx); > + > + if (total_len > *size) > + rc = -E2BIG; > + else > + lsm_fill_user_ctx(ctx, value, len, LSM_ID_SELINUX, 0); > + > + *size = total_len; > + return rc; > +} > + > +static int selinux_setselfattr(unsigned int __user attr, > + struct lsm_ctx __user *ctx, size_t __user size, > + u32 __user flags) > +{ > + struct lsm_ctx *lctx; > + void *context; > + int rc; > + > + context = kmalloc(size, GFP_KERNEL); > + if (context == NULL) > + return -ENOMEM; > + > + lctx = (struct lsm_ctx *)context; > + if (copy_from_user(context, ctx, size)) > + rc = -EFAULT; > + else if (lctx->ctx_len > size) > + rc = -EINVAL; > + else > + rc = do_setattr(attr, lctx + 1, lctx->ctx_len); > + > + kfree(context); > + if (rc > 0) > + return 0; > + return rc; > +} > + > +static int selinux_getprocattr(struct task_struct *p, > + const char *name, char **value) > +{ > + unsigned int attr = lsm_name_to_attr(name); > + > + if (attr) > + return do_getattr(attr, p, value); > + return -EINVAL; > +} > + > +static int selinux_setprocattr(const char *name, void *value, size_t size) > +{ > + int attr = lsm_name_to_attr(name); > + > + if (attr) > + return do_setattr(attr, value, size); > + return -EINVAL; > +} > + > static int selinux_ismaclabel(const char *name) > { > return (strcmp(name, XATTR_SELINUX_SUFFIX) == 0); > @@ -7183,6 +7268,8 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { > > LSM_HOOK_INIT(d_instantiate, selinux_d_instantiate), > > + LSM_HOOK_INIT(getselfattr, selinux_getselfattr), > + LSM_HOOK_INIT(setselfattr, selinux_setselfattr), > LSM_HOOK_INIT(getprocattr, selinux_getprocattr), > LSM_HOOK_INIT(setprocattr, selinux_setprocattr), > > -- > 2.39.2 -- paul-moore.com
On 3/29/2023 6:13 PM, Paul Moore wrote: > On Wed, Mar 15, 2023 at 6:52 PM Casey Schaufler <casey@schaufler-ca.com> wrote: >> Add hooks for setselfattr and getselfattr. These hooks are not very >> different from their setprocattr and getprocattr equivalents, and >> much of the code is shared. >> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> >> Cc: selinux@vger.kernel.org >> Cc: Paul Moore <paul@paul-moore.com> >> --- >> security/selinux/hooks.c | 147 +++++++++++++++++++++++++++++++-------- >> 1 file changed, 117 insertions(+), 30 deletions(-) >> >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c >> index 9403aee75981..8896edf80aa9 100644 >> --- a/security/selinux/hooks.c >> +++ b/security/selinux/hooks.c >> @@ -6348,8 +6348,7 @@ static void selinux_d_instantiate(struct dentry *dentry, struct inode *inode) >> inode_doinit_with_dentry(inode, dentry); >> } >> >> -static int selinux_getprocattr(struct task_struct *p, >> - const char *name, char **value) >> +static int do_getattr(unsigned int attr, struct task_struct *p, char **value) > Are you ready for more naming nitpicks? ;) I would expect nothing less. :) > Let's call this 'selinux_lsm_getattr()', and the matching setter > should be 'selinux_lsm_setattr()'. As you wish. It's your LSM. >> { >> const struct task_security_struct *__tsec; >> u32 sid; >> @@ -6367,20 +6366,27 @@ static int selinux_getprocattr(struct task_struct *p, >> goto bad; >> } >> >> - if (!strcmp(name, "current")) >> + switch (attr) { >> + case LSM_ATTR_CURRENT: >> sid = __tsec->sid; >> - else if (!strcmp(name, "prev")) >> + break; >> + case LSM_ATTR_PREV: >> sid = __tsec->osid; >> - else if (!strcmp(name, "exec")) >> + break; >> + case LSM_ATTR_EXEC: >> sid = __tsec->exec_sid; >> - else if (!strcmp(name, "fscreate")) >> + break; >> + case LSM_ATTR_FSCREATE: >> sid = __tsec->create_sid; >> - else if (!strcmp(name, "keycreate")) >> + break; >> + case LSM_ATTR_KEYCREATE: >> sid = __tsec->keycreate_sid; >> - else if (!strcmp(name, "sockcreate")) >> + break; >> + case LSM_ATTR_SOCKCREATE: >> sid = __tsec->sockcreate_sid; >> - else { >> - error = -EINVAL; >> + break; >> + default: >> + error = -EOPNOTSUPP; > The error should probably be -EINVAL. It's possible that we may add an attribute that SELinux doesn't support, say LSM_ATTR_CRYPTO_KEY, that another LSM does. This is the same behavior the other LSMs exhibit in the face of a request for attributes such as LSM_ATTR_KEYCREATE that they don't support. >> goto bad; >> } >> rcu_read_unlock(); >> @@ -6398,7 +6404,7 @@ static int selinux_getprocattr(struct task_struct *p, >> return error; >> } >> >> -static int selinux_setprocattr(const char *name, void *value, size_t size) >> +static int do_setattr(u64 attr, void *value, size_t size) >> { >> struct task_security_struct *tsec; >> struct cred *new; >> @@ -6409,28 +6415,36 @@ static int selinux_setprocattr(const char *name, void *value, size_t size) >> /* >> * Basic control over ability to set these attributes at all. >> */ >> - if (!strcmp(name, "exec")) >> + switch (attr) { >> + case LSM_ATTR_CURRENT: >> + error = avc_has_perm(&selinux_state, >> + mysid, mysid, SECCLASS_PROCESS, >> + PROCESS__SETCURRENT, NULL); >> + break; >> + case LSM_ATTR_EXEC: >> error = avc_has_perm(&selinux_state, >> mysid, mysid, SECCLASS_PROCESS, >> PROCESS__SETEXEC, NULL); >> - else if (!strcmp(name, "fscreate")) >> + break; >> + case LSM_ATTR_FSCREATE: >> error = avc_has_perm(&selinux_state, >> mysid, mysid, SECCLASS_PROCESS, >> PROCESS__SETFSCREATE, NULL); >> - else if (!strcmp(name, "keycreate")) >> + break; >> + case LSM_ATTR_KEYCREATE: >> error = avc_has_perm(&selinux_state, >> mysid, mysid, SECCLASS_PROCESS, >> PROCESS__SETKEYCREATE, NULL); >> - else if (!strcmp(name, "sockcreate")) >> + break; >> + case LSM_ATTR_SOCKCREATE: >> error = avc_has_perm(&selinux_state, >> mysid, mysid, SECCLASS_PROCESS, >> PROCESS__SETSOCKCREATE, NULL); >> - else if (!strcmp(name, "current")) >> - error = avc_has_perm(&selinux_state, >> - mysid, mysid, SECCLASS_PROCESS, >> - PROCESS__SETCURRENT, NULL); >> - else >> - error = -EINVAL; >> + break; >> + default: >> + error = -EOPNOTSUPP; > Same as above, should be -EINVAL. Same as above, there may be attributes SELinux doesn't support. >> + break; >> + } >> if (error) >> return error; >> >> @@ -6442,13 +6456,14 @@ static int selinux_setprocattr(const char *name, void *value, size_t size) >> } >> error = security_context_to_sid(&selinux_state, value, size, >> &sid, GFP_KERNEL); >> - if (error == -EINVAL && !strcmp(name, "fscreate")) { >> + if (error == -EINVAL && attr == LSM_ATTR_FSCREATE) { >> if (!has_cap_mac_admin(true)) { >> struct audit_buffer *ab; >> size_t audit_size; >> >> - /* We strip a nul only if it is at the end, otherwise the >> - * context contains a nul and we should audit that */ >> + /* We strip a nul only if it is at the end, >> + * otherwise the context contains a nul and >> + * we should audit that */ > You *do* get gold stars for fixing line lengths in close proximity ;) I guess I'm the Last User of the 80 character terminal. >> if (str[size - 1] == '\0') >> audit_size = size - 1; >> else >> @@ -6459,7 +6474,8 @@ static int selinux_setprocattr(const char *name, void *value, size_t size) >> if (!ab) >> return error; >> audit_log_format(ab, "op=fscreate invalid_context="); >> - audit_log_n_untrustedstring(ab, value, audit_size); >> + audit_log_n_untrustedstring(ab, value, >> + audit_size); >> audit_log_end(ab); >> >> return error; >> @@ -6483,11 +6499,11 @@ static int selinux_setprocattr(const char *name, void *value, size_t size) >> checks and may_create for the file creation checks. The >> operation will then fail if the context is not permitted. */ >> tsec = selinux_cred(new); >> - if (!strcmp(name, "exec")) { >> + if (attr == LSM_ATTR_EXEC) { >> tsec->exec_sid = sid; >> - } else if (!strcmp(name, "fscreate")) { >> + } else if (attr == LSM_ATTR_FSCREATE) { >> tsec->create_sid = sid; >> - } else if (!strcmp(name, "keycreate")) { >> + } else if (attr == LSM_ATTR_KEYCREATE) { >> if (sid) { >> error = avc_has_perm(&selinux_state, mysid, sid, >> SECCLASS_KEY, KEY__CREATE, NULL); >> @@ -6495,9 +6511,9 @@ static int selinux_setprocattr(const char *name, void *value, size_t size) >> goto abort_change; >> } >> tsec->keycreate_sid = sid; >> - } else if (!strcmp(name, "sockcreate")) { >> + } else if (attr == LSM_ATTR_SOCKCREATE) { >> tsec->sockcreate_sid = sid; >> - } else if (!strcmp(name, "current")) { >> + } else if (attr == LSM_ATTR_CURRENT) { >> error = -EINVAL; >> if (sid == 0) >> goto abort_change; >> @@ -6542,6 +6558,75 @@ static int selinux_setprocattr(const char *name, void *value, size_t size) >> return error; >> } >> >> +static int selinux_getselfattr(unsigned int __user attr, >> + struct lsm_ctx __user *ctx, size_t *size, >> + u32 __user flags) >> +{ >> + char *value; >> + size_t total_len; >> + int len; >> + int rc = 0; >> + >> + len = do_getattr(attr, current, &value); >> + if (len < 0) >> + return len; >> + >> + total_len = len + sizeof(*ctx); >> + >> + if (total_len > *size) >> + rc = -E2BIG; >> + else >> + lsm_fill_user_ctx(ctx, value, len, LSM_ID_SELINUX, 0); >> + >> + *size = total_len; >> + return rc; >> +} >> + >> +static int selinux_setselfattr(unsigned int __user attr, >> + struct lsm_ctx __user *ctx, size_t __user size, >> + u32 __user flags) >> +{ >> + struct lsm_ctx *lctx; >> + void *context; >> + int rc; >> + >> + context = kmalloc(size, GFP_KERNEL); >> + if (context == NULL) >> + return -ENOMEM; >> + >> + lctx = (struct lsm_ctx *)context; >> + if (copy_from_user(context, ctx, size)) >> + rc = -EFAULT; >> + else if (lctx->ctx_len > size) >> + rc = -EINVAL; >> + else >> + rc = do_setattr(attr, lctx + 1, lctx->ctx_len); >> + >> + kfree(context); >> + if (rc > 0) >> + return 0; >> + return rc; >> +} >> + >> +static int selinux_getprocattr(struct task_struct *p, >> + const char *name, char **value) >> +{ >> + unsigned int attr = lsm_name_to_attr(name); >> + >> + if (attr) >> + return do_getattr(attr, p, value); >> + return -EINVAL; >> +} >> + >> +static int selinux_setprocattr(const char *name, void *value, size_t size) >> +{ >> + int attr = lsm_name_to_attr(name); >> + >> + if (attr) >> + return do_setattr(attr, value, size); >> + return -EINVAL; >> +} >> + >> static int selinux_ismaclabel(const char *name) >> { >> return (strcmp(name, XATTR_SELINUX_SUFFIX) == 0); >> @@ -7183,6 +7268,8 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { >> >> LSM_HOOK_INIT(d_instantiate, selinux_d_instantiate), >> >> + LSM_HOOK_INIT(getselfattr, selinux_getselfattr), >> + LSM_HOOK_INIT(setselfattr, selinux_setselfattr), >> LSM_HOOK_INIT(getprocattr, selinux_getprocattr), >> LSM_HOOK_INIT(setprocattr, selinux_setprocattr), >> >> -- >> 2.39.2 > -- > paul-moore.com
On Thu, Mar 30, 2023 at 4:55 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > On 3/29/2023 6:13 PM, Paul Moore wrote: > > On Wed, Mar 15, 2023 at 6:52 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > >> Add hooks for setselfattr and getselfattr. These hooks are not very > >> different from their setprocattr and getprocattr equivalents, and > >> much of the code is shared. > >> > >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > >> Cc: selinux@vger.kernel.org > >> Cc: Paul Moore <paul@paul-moore.com> > >> --- > >> security/selinux/hooks.c | 147 +++++++++++++++++++++++++++++++-------- > >> 1 file changed, 117 insertions(+), 30 deletions(-) > >> > >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > >> index 9403aee75981..8896edf80aa9 100644 > >> --- a/security/selinux/hooks.c > >> +++ b/security/selinux/hooks.c > >> @@ -6348,8 +6348,7 @@ static void selinux_d_instantiate(struct dentry *dentry, struct inode *inode) > >> inode_doinit_with_dentry(inode, dentry); > >> } > >> > >> -static int selinux_getprocattr(struct task_struct *p, > >> - const char *name, char **value) > >> +static int do_getattr(unsigned int attr, struct task_struct *p, char **value) > > Are you ready for more naming nitpicks? ;) > > I would expect nothing less. :) > > > Let's call this 'selinux_lsm_getattr()', and the matching setter > > should be 'selinux_lsm_setattr()'. > > As you wish. It's your LSM. > > > >> { > >> const struct task_security_struct *__tsec; > >> u32 sid; > >> @@ -6367,20 +6366,27 @@ static int selinux_getprocattr(struct task_struct *p, > >> goto bad; > >> } > >> > >> - if (!strcmp(name, "current")) > >> + switch (attr) { > >> + case LSM_ATTR_CURRENT: > >> sid = __tsec->sid; > >> - else if (!strcmp(name, "prev")) > >> + break; > >> + case LSM_ATTR_PREV: > >> sid = __tsec->osid; > >> - else if (!strcmp(name, "exec")) > >> + break; > >> + case LSM_ATTR_EXEC: > >> sid = __tsec->exec_sid; > >> - else if (!strcmp(name, "fscreate")) > >> + break; > >> + case LSM_ATTR_FSCREATE: > >> sid = __tsec->create_sid; > >> - else if (!strcmp(name, "keycreate")) > >> + break; > >> + case LSM_ATTR_KEYCREATE: > >> sid = __tsec->keycreate_sid; > >> - else if (!strcmp(name, "sockcreate")) > >> + break; > >> + case LSM_ATTR_SOCKCREATE: > >> sid = __tsec->sockcreate_sid; > >> - else { > >> - error = -EINVAL; > >> + break; > >> + default: > >> + error = -EOPNOTSUPP; > > The error should probably be -EINVAL. > > It's possible that we may add an attribute that SELinux doesn't > support, say LSM_ATTR_CRYPTO_KEY, that another LSM does. This is > the same behavior the other LSMs exhibit in the face of a request > for attributes such as LSM_ATTR_KEYCREATE that they don't support. Okay, I'll accept that argument, but I would ask that add some additional handling in selinux_getprocattr() so that it returns -EINVAL in this case just as it does today. > >> goto bad; > >> } > >> rcu_read_unlock(); > >> @@ -6398,7 +6404,7 @@ static int selinux_getprocattr(struct task_struct *p, > >> return error; > >> } > >> > >> -static int selinux_setprocattr(const char *name, void *value, size_t size) > >> +static int do_setattr(u64 attr, void *value, size_t size) > >> { > >> struct task_security_struct *tsec; > >> struct cred *new; > >> @@ -6409,28 +6415,36 @@ static int selinux_setprocattr(const char *name, void *value, size_t size) > >> /* > >> * Basic control over ability to set these attributes at all. > >> */ > >> - if (!strcmp(name, "exec")) > >> + switch (attr) { > >> + case LSM_ATTR_CURRENT: > >> + error = avc_has_perm(&selinux_state, > >> + mysid, mysid, SECCLASS_PROCESS, > >> + PROCESS__SETCURRENT, NULL); > >> + break; > >> + case LSM_ATTR_EXEC: > >> error = avc_has_perm(&selinux_state, > >> mysid, mysid, SECCLASS_PROCESS, > >> PROCESS__SETEXEC, NULL); > >> - else if (!strcmp(name, "fscreate")) > >> + break; > >> + case LSM_ATTR_FSCREATE: > >> error = avc_has_perm(&selinux_state, > >> mysid, mysid, SECCLASS_PROCESS, > >> PROCESS__SETFSCREATE, NULL); > >> - else if (!strcmp(name, "keycreate")) > >> + break; > >> + case LSM_ATTR_KEYCREATE: > >> error = avc_has_perm(&selinux_state, > >> mysid, mysid, SECCLASS_PROCESS, > >> PROCESS__SETKEYCREATE, NULL); > >> - else if (!strcmp(name, "sockcreate")) > >> + break; > >> + case LSM_ATTR_SOCKCREATE: > >> error = avc_has_perm(&selinux_state, > >> mysid, mysid, SECCLASS_PROCESS, > >> PROCESS__SETSOCKCREATE, NULL); > >> - else if (!strcmp(name, "current")) > >> - error = avc_has_perm(&selinux_state, > >> - mysid, mysid, SECCLASS_PROCESS, > >> - PROCESS__SETCURRENT, NULL); > >> - else > >> - error = -EINVAL; > >> + break; > >> + default: > >> + error = -EOPNOTSUPP; > > Same as above, should be -EINVAL. > > Same as above, there may be attributes SELinux doesn't support. Also, same as above. > >> + break; > >> + } > >> if (error) > >> return error; > >> > >> @@ -6442,13 +6456,14 @@ static int selinux_setprocattr(const char *name, void *value, size_t size) > >> } > >> error = security_context_to_sid(&selinux_state, value, size, > >> &sid, GFP_KERNEL); > >> - if (error == -EINVAL && !strcmp(name, "fscreate")) { > >> + if (error == -EINVAL && attr == LSM_ATTR_FSCREATE) { > >> if (!has_cap_mac_admin(true)) { > >> struct audit_buffer *ab; > >> size_t audit_size; > >> > >> - /* We strip a nul only if it is at the end, otherwise the > >> - * context contains a nul and we should audit that */ > >> + /* We strip a nul only if it is at the end, > >> + * otherwise the context contains a nul and > >> + * we should audit that */ > > You *do* get gold stars for fixing line lengths in close proximity ;) > > I guess I'm the Last User of the 80 character terminal. I'm still a big fan and I'm sticking to the 80 char limit for the LSM layer as well as the SELinux, audit, and labeled networking subsystems. Longer lines either predate me or I simply didn't catch them during review/merge.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 9403aee75981..8896edf80aa9 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6348,8 +6348,7 @@ static void selinux_d_instantiate(struct dentry *dentry, struct inode *inode) inode_doinit_with_dentry(inode, dentry); } -static int selinux_getprocattr(struct task_struct *p, - const char *name, char **value) +static int do_getattr(unsigned int attr, struct task_struct *p, char **value) { const struct task_security_struct *__tsec; u32 sid; @@ -6367,20 +6366,27 @@ static int selinux_getprocattr(struct task_struct *p, goto bad; } - if (!strcmp(name, "current")) + switch (attr) { + case LSM_ATTR_CURRENT: sid = __tsec->sid; - else if (!strcmp(name, "prev")) + break; + case LSM_ATTR_PREV: sid = __tsec->osid; - else if (!strcmp(name, "exec")) + break; + case LSM_ATTR_EXEC: sid = __tsec->exec_sid; - else if (!strcmp(name, "fscreate")) + break; + case LSM_ATTR_FSCREATE: sid = __tsec->create_sid; - else if (!strcmp(name, "keycreate")) + break; + case LSM_ATTR_KEYCREATE: sid = __tsec->keycreate_sid; - else if (!strcmp(name, "sockcreate")) + break; + case LSM_ATTR_SOCKCREATE: sid = __tsec->sockcreate_sid; - else { - error = -EINVAL; + break; + default: + error = -EOPNOTSUPP; goto bad; } rcu_read_unlock(); @@ -6398,7 +6404,7 @@ static int selinux_getprocattr(struct task_struct *p, return error; } -static int selinux_setprocattr(const char *name, void *value, size_t size) +static int do_setattr(u64 attr, void *value, size_t size) { struct task_security_struct *tsec; struct cred *new; @@ -6409,28 +6415,36 @@ static int selinux_setprocattr(const char *name, void *value, size_t size) /* * Basic control over ability to set these attributes at all. */ - if (!strcmp(name, "exec")) + switch (attr) { + case LSM_ATTR_CURRENT: + error = avc_has_perm(&selinux_state, + mysid, mysid, SECCLASS_PROCESS, + PROCESS__SETCURRENT, NULL); + break; + case LSM_ATTR_EXEC: error = avc_has_perm(&selinux_state, mysid, mysid, SECCLASS_PROCESS, PROCESS__SETEXEC, NULL); - else if (!strcmp(name, "fscreate")) + break; + case LSM_ATTR_FSCREATE: error = avc_has_perm(&selinux_state, mysid, mysid, SECCLASS_PROCESS, PROCESS__SETFSCREATE, NULL); - else if (!strcmp(name, "keycreate")) + break; + case LSM_ATTR_KEYCREATE: error = avc_has_perm(&selinux_state, mysid, mysid, SECCLASS_PROCESS, PROCESS__SETKEYCREATE, NULL); - else if (!strcmp(name, "sockcreate")) + break; + case LSM_ATTR_SOCKCREATE: error = avc_has_perm(&selinux_state, mysid, mysid, SECCLASS_PROCESS, PROCESS__SETSOCKCREATE, NULL); - else if (!strcmp(name, "current")) - error = avc_has_perm(&selinux_state, - mysid, mysid, SECCLASS_PROCESS, - PROCESS__SETCURRENT, NULL); - else - error = -EINVAL; + break; + default: + error = -EOPNOTSUPP; + break; + } if (error) return error; @@ -6442,13 +6456,14 @@ static int selinux_setprocattr(const char *name, void *value, size_t size) } error = security_context_to_sid(&selinux_state, value, size, &sid, GFP_KERNEL); - if (error == -EINVAL && !strcmp(name, "fscreate")) { + if (error == -EINVAL && attr == LSM_ATTR_FSCREATE) { if (!has_cap_mac_admin(true)) { struct audit_buffer *ab; size_t audit_size; - /* We strip a nul only if it is at the end, otherwise the - * context contains a nul and we should audit that */ + /* We strip a nul only if it is at the end, + * otherwise the context contains a nul and + * we should audit that */ if (str[size - 1] == '\0') audit_size = size - 1; else @@ -6459,7 +6474,8 @@ static int selinux_setprocattr(const char *name, void *value, size_t size) if (!ab) return error; audit_log_format(ab, "op=fscreate invalid_context="); - audit_log_n_untrustedstring(ab, value, audit_size); + audit_log_n_untrustedstring(ab, value, + audit_size); audit_log_end(ab); return error; @@ -6483,11 +6499,11 @@ static int selinux_setprocattr(const char *name, void *value, size_t size) checks and may_create for the file creation checks. The operation will then fail if the context is not permitted. */ tsec = selinux_cred(new); - if (!strcmp(name, "exec")) { + if (attr == LSM_ATTR_EXEC) { tsec->exec_sid = sid; - } else if (!strcmp(name, "fscreate")) { + } else if (attr == LSM_ATTR_FSCREATE) { tsec->create_sid = sid; - } else if (!strcmp(name, "keycreate")) { + } else if (attr == LSM_ATTR_KEYCREATE) { if (sid) { error = avc_has_perm(&selinux_state, mysid, sid, SECCLASS_KEY, KEY__CREATE, NULL); @@ -6495,9 +6511,9 @@ static int selinux_setprocattr(const char *name, void *value, size_t size) goto abort_change; } tsec->keycreate_sid = sid; - } else if (!strcmp(name, "sockcreate")) { + } else if (attr == LSM_ATTR_SOCKCREATE) { tsec->sockcreate_sid = sid; - } else if (!strcmp(name, "current")) { + } else if (attr == LSM_ATTR_CURRENT) { error = -EINVAL; if (sid == 0) goto abort_change; @@ -6542,6 +6558,75 @@ static int selinux_setprocattr(const char *name, void *value, size_t size) return error; } +static int selinux_getselfattr(unsigned int __user attr, + struct lsm_ctx __user *ctx, size_t *size, + u32 __user flags) +{ + char *value; + size_t total_len; + int len; + int rc = 0; + + len = do_getattr(attr, current, &value); + if (len < 0) + return len; + + total_len = len + sizeof(*ctx); + + if (total_len > *size) + rc = -E2BIG; + else + lsm_fill_user_ctx(ctx, value, len, LSM_ID_SELINUX, 0); + + *size = total_len; + return rc; +} + +static int selinux_setselfattr(unsigned int __user attr, + struct lsm_ctx __user *ctx, size_t __user size, + u32 __user flags) +{ + struct lsm_ctx *lctx; + void *context; + int rc; + + context = kmalloc(size, GFP_KERNEL); + if (context == NULL) + return -ENOMEM; + + lctx = (struct lsm_ctx *)context; + if (copy_from_user(context, ctx, size)) + rc = -EFAULT; + else if (lctx->ctx_len > size) + rc = -EINVAL; + else + rc = do_setattr(attr, lctx + 1, lctx->ctx_len); + + kfree(context); + if (rc > 0) + return 0; + return rc; +} + +static int selinux_getprocattr(struct task_struct *p, + const char *name, char **value) +{ + unsigned int attr = lsm_name_to_attr(name); + + if (attr) + return do_getattr(attr, p, value); + return -EINVAL; +} + +static int selinux_setprocattr(const char *name, void *value, size_t size) +{ + int attr = lsm_name_to_attr(name); + + if (attr) + return do_setattr(attr, value, size); + return -EINVAL; +} + static int selinux_ismaclabel(const char *name) { return (strcmp(name, XATTR_SELINUX_SUFFIX) == 0); @@ -7183,6 +7268,8 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(d_instantiate, selinux_d_instantiate), + LSM_HOOK_INIT(getselfattr, selinux_getselfattr), + LSM_HOOK_INIT(setselfattr, selinux_setselfattr), LSM_HOOK_INIT(getprocattr, selinux_getprocattr), LSM_HOOK_INIT(setprocattr, selinux_setprocattr),
Add hooks for setselfattr and getselfattr. These hooks are not very different from their setprocattr and getprocattr equivalents, and much of the code is shared. Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> Cc: selinux@vger.kernel.org Cc: Paul Moore <paul@paul-moore.com> --- security/selinux/hooks.c | 147 +++++++++++++++++++++++++++++++-------- 1 file changed, 117 insertions(+), 30 deletions(-)