[v7,10/11] SELinux: Add selfattr hooks
Commit Message
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(-)
Comments
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.
@@ -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),