Re: [PATCH v7 04/11] LSM: syscalls for current process attributes
From: Mickaël Salaün
Date: Mon Apr 03 2023 - 08:04:14 EST
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@xxxxxxxxxxxxxxxx>
---
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@xxxxxxxxxxxxxxxx>
+ * 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)
{