[PATCH v7 04/11] LSM: syscalls for current process attributes
Mickaël Salaün
mic at digikod.net
Mon Apr 3 12:04:00 UTC 2023
On 15/03/2023 23:46, Casey Schaufler wrote:
> Create a system call lsm_get_self_attr() to provide the security
> module maintained attributes of the current process.
> Create a system call lsm_set_self_attr() to set a security
> module maintained attribute of the current process.
> Historically these attributes have been exposed to user space via
> entries in procfs under /proc/self/attr.
>
> The attribute value is provided in a lsm_ctx structure. The structure
> identifys the size of the attribute, and the attribute value. The format
> of the attribute value is defined by the security module. A flags field
> is included for LSM specific information. It is currently unused and must
> be 0. The total size of the data, including the lsm_ctx structure and any
> padding, is maintained as well.
>
> struct lsm_ctx {
> __u64 id;
> __u64 flags;
> __u64 len;
> __u64 ctx_len;
> __u8 ctx[];
> };
>
> Two new LSM hooks are used to interface with the LSMs.
> security_getselfattr() collects the lsm_ctx values from the
> LSMs that support the hook, accounting for space requirements.
> security_setselfattr() identifies which LSM the attribute is
> intended for and passes it along.
>
> Signed-off-by: Casey Schaufler <casey at schaufler-ca.com>
> ---
> Documentation/userspace-api/lsm.rst | 15 +++++
> include/linux/lsm_hook_defs.h | 4 ++
> include/linux/lsm_hooks.h | 9 +++
> include/linux/security.h | 19 ++++++
> include/linux/syscalls.h | 5 ++
> include/uapi/linux/lsm.h | 33 ++++++++++
> kernel/sys_ni.c | 4 ++
> security/Makefile | 1 +
> security/lsm_syscalls.c | 55 ++++++++++++++++
> security/security.c | 97 +++++++++++++++++++++++++++++
> 10 files changed, 242 insertions(+)
> create mode 100644 security/lsm_syscalls.c
[...]
> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
> new file mode 100644
> index 000000000000..feee31600219
> --- /dev/null
> +++ b/security/lsm_syscalls.c
> @@ -0,0 +1,55 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * System calls implementing the Linux Security Module API.
> + *
> + * Copyright (C) 2022 Casey Schaufler <casey at schaufler-ca.com>
> + * Copyright (C) 2022 Intel Corporation
> + */
> +
> +#include <asm/current.h>
> +#include <linux/compiler_types.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/security.h>
> +#include <linux/stddef.h>
> +#include <linux/syscalls.h>
> +#include <linux/types.h>
> +#include <linux/lsm_hooks.h>
> +#include <uapi/linux/lsm.h>
> +
> +/**
> + * sys_lsm_set_self_attr - Set current task's security module attribute
> + * @attr: which attribute to set
> + * @ctx: the LSM contexts
> + * @size: size of @ctx
> + * @flags: reserved for future use
> + *
> + * Sets the calling task's LSM context. On success this function
> + * returns 0. If the attribute specified cannot be set a negative
> + * value indicating the reason for the error is returned.
Do you think it is really worth it to implement syscalls that can get
and set attributes to several LSMs at the same time, instead of one at a
time? LSM-specific tools don't care about other LSMs. I still think it
would be much simpler (for kernel and user space) to pass an LSM ID to
both syscalls. This would avoid dealing with variable arrays of variable
element lengths, to both get or set attributes.
Furthermore, considering the hypotetical LSM_ATTR_MAGICFD that was
previously talked about, getting an unknown number of file descriptor
doesn't look good neither.
> + */
> +SYSCALL_DEFINE4(lsm_set_self_attr, unsigned int, attr, struct lsm_ctx __user *,
> + ctx, size_t __user, size, u32, flags)
> +{
> + return security_setselfattr(attr, ctx, size, flags);
> +}
> +
> +/**
> + * sys_lsm_get_self_attr - Return current task's security module attributes
> + * @attr: which attribute to set
attribute to *get*
> + * @ctx: the LSM contexts
> + * @size: size of @ctx, updated on return
I suggest to use a dedicated argument to read the allocated size, and
another to write the actual/written size.
This would not be required with an LSM ID passed to the syscall because
attribute sizes should be known by user space, and there is no need to
help them probe this information.
> + * @flags: reserved for future use
> + *
> + * Returns the calling task's LSM contexts. On success this
> + * function returns the number of @ctx array elements. This value
> + * may be zero if there are no LSM contexts assigned. If @size is
> + * insufficient to contain the return data -E2BIG is returned and
> + * @size is set to the minimum required size.
Doing something (updating a buffer) even when returning an error doesn't
look right. These sizes should be well-known to user space and part of
the ABI/UAPI.
> In all other cases
> + * a negative value indicating the error is returned.
> + */
> +SYSCALL_DEFINE4(lsm_get_self_attr, unsigned int, attr, struct lsm_ctx __user *,
> + ctx, size_t __user *, size, u32, flags)
> +{
> + return security_getselfattr(attr, ctx, size, flags);
> +}
> diff --git a/security/security.c b/security/security.c
> index 87c8796c3c46..2c57fe28c4f7 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2168,6 +2168,103 @@ void security_d_instantiate(struct dentry *dentry, struct inode *inode)
> }
> EXPORT_SYMBOL(security_d_instantiate);
>
> +/**
> + * security_getselfattr - Read an LSM attribute of the current process.
> + * @attr: which attribute to return
> + * @ctx: the user-space destination for the information, or NULL
> + * @size: the size of space available to receive the data
> + * @flags: reserved for future use, must be 0
> + *
> + * Returns the number of attributes found on success, negative value
> + * on error. @size is reset to the total size of the data.
> + * If @size is insufficient to contain the data -E2BIG is returned.
> + */
> +int security_getselfattr(unsigned int __user attr, struct lsm_ctx __user *ctx,
> + size_t __user *size, u32 __user flags)
> +{
> + struct security_hook_list *hp;
> + void __user *base = (void *)ctx;
> + size_t total = 0;
> + size_t this;
> + size_t left;
> + bool istoobig = false;
> + int count = 0;
> + int rc;
> +
> + if (attr == 0)
> + return -EINVAL;
> + if (flags != 0)
> + return -EINVAL;
> + if (size == NULL)
> + return -EINVAL;
> + if (get_user(left, size))
> + return -EFAULT;
> +
> + hlist_for_each_entry(hp, &security_hook_heads.getselfattr, list) {
> + this = left;
> + if (base)
> + ctx = (struct lsm_ctx __user *)(base + total);
> + rc = hp->hook.getselfattr(attr, ctx, &this, flags);
> + switch (rc) {
> + case -EOPNOTSUPP:
> + rc = 0;
> + continue;
> + case -E2BIG:
> + istoobig = true;
> + left = 0;
> + break;
These two error cases could be directly handled by
security_getselfattr() instead of relying on each LSM-specific
implementations. See my suggestion on patch 7/11 (lsm_get_attr_size).
> + case 0:
> + left -= this;
> + break;
> + default:
> + return rc;
> + }
> + total += this;
> + count++;
> + }
> + if (count == 0)
> + return LSM_RET_DEFAULT(getselfattr);
> + if (put_user(total, size))
> + return -EFAULT;
> + if (rc)
> + return rc;
> + if (istoobig)
> + return -E2BIG;
> + return count;
> +}
> +
> +/**
> + * security_setselfattr - Set an LSM attribute on the current process.
> + * @attr: which attribute to set
> + * @ctx: the user-space source for the information
> + * @size: the size of the data
> + * @flags: reserved for future use, must be 0
> + *
> + * Set an LSM attribute for the current process. The LSM, attribute
> + * and new value are included in @ctx.
> + *
> + * Returns 0 on success, an LSM specific value on failure.
> + */
> +int security_setselfattr(unsigned int __user attr, struct lsm_ctx __user *ctx,
> + size_t __user size, u32 __user flags)
> +{
> + struct security_hook_list *hp;
> + struct lsm_ctx lctx;
> +
> + if (flags != 0)
> + return -EINVAL;
> + if (size < sizeof(*ctx))
> + return -EINVAL;
> + if (copy_from_user(&lctx, ctx, sizeof(*ctx)))
> + return -EFAULT;
> +
> + hlist_for_each_entry(hp, &security_hook_heads.setselfattr, list)
> + if ((hp->lsmid->id) == lctx.id)
> + return hp->hook.setselfattr(attr, ctx, size, flags);
> +
> + return LSM_RET_DEFAULT(setselfattr);
> +}
> +
> int security_getprocattr(struct task_struct *p, int lsmid, const char *name,
> char **value)
> {
More information about the Linux-security-module-archive
mailing list