On Wed, Mar 15, 2023 at 6:52 PM Casey Schaufler <casey@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxx> > Cc: selinux@xxxxxxxxxxxxxxx > Cc: Paul Moore <paul@xxxxxxxxxxxxxx> > --- > 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