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)
{
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),
--
2.39.2On 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.
--
paul-moore.com
© 2016 - 2026 Red Hat, Inc.