Re: [PATCH v7 07/11] LSM: Helpers for attribute names and filling an lsm_ctx
From: Paul Moore <paul@paul-moore.com>
Date: 2023-03-30 01:13:42
Also in:
linux-security-module, lkml
On Wed, Mar 15, 2023 at 6:50 PM Casey Schaufler [off-list ref] wrote:
Add lsm_name_to_attr(), which translates a text string to a LSM_ATTR value if one is available. Add lsm_fill_user_ctx(), which fills a struct lsm_ctx, including the trailing attribute value. All are used in module specific components of LSM system calls. Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> --- include/linux/security.h | 13 ++++++++++ security/lsm_syscalls.c | 51 ++++++++++++++++++++++++++++++++++++++++ security/security.c | 31 ++++++++++++++++++++++++ 3 files changed, 95 insertions(+)
...
quoted hunk ↗ jump to hunk
diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c index 6efbe244d304..55d849ad5d6e 100644 --- a/security/lsm_syscalls.c +++ b/security/lsm_syscalls.c@@ -17,6 +17,57 @@ #include <linux/lsm_hooks.h> #include <uapi/linux/lsm.h> +struct attr_map { + char *name; + u64 attr; +}; + +static const struct attr_map lsm_attr_names[] = { + { + .name = "current", + .attr = LSM_ATTR_CURRENT, + }, + { + .name = "exec", + .attr = LSM_ATTR_EXEC, + }, + { + .name = "fscreate", + .attr = LSM_ATTR_FSCREATE, + }, + { + .name = "keycreate", + .attr = LSM_ATTR_KEYCREATE, + }, + { + .name = "prev", + .attr = LSM_ATTR_PREV, + }, + { + .name = "sockcreate", + .attr = LSM_ATTR_SOCKCREATE, + }, +}; + +/** + * lsm_name_to_attr - map an LSM attribute name to its ID + * @name: name of the attribute + * + * Look the given @name up in the table of know attribute names. + * + * Returns the LSM attribute value associated with @name, or 0 if + * there is no mapping. + */ +u64 lsm_name_to_attr(const char *name) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(lsm_attr_names); i++) + if (!strcmp(name, lsm_attr_names[i].name)) + return lsm_attr_names[i].attr;
I'm pretty sure this is the only place where @lsm_attr_names is used, right? If true, when coupled with the idea that these syscalls are going to close the door on new LSM attributes in procfs I think we can just put the mapping directly in this function via a series of if-statements.
quoted hunk ↗ jump to hunk
+ return 0; +} + /** * sys_lsm_set_self_attr - Set current task's security module attribute * @attr: which attribute to setdiff --git a/security/security.c b/security/security.c index 2c57fe28c4f7..f7b814a3940c 100644 --- a/security/security.c +++ b/security/security.c@@ -753,6 +753,37 @@ static int lsm_superblock_alloc(struct super_block *sb) return 0; } +/** + * lsm_fill_user_ctx - Fill a user space lsm_ctx structure + * @ctx: an LSM context to be filled + * @context: the new context value + * @context_size: the size of the new context value + * @id: LSM id + * @flags: LSM defined flags + * + * Fill all of the fields in a user space lsm_ctx structure. + * Caller is assumed to have verified that @ctx has enough space + * for @context. + * Returns 0 on success, -EFAULT on a copyout error. + */ +int lsm_fill_user_ctx(struct lsm_ctx __user *ctx, void *context, + size_t context_size, u64 id, u64 flags) +{ + struct lsm_ctx local; + void __user *vc = ctx; + + local.id = id; + local.flags = flags; + local.ctx_len = context_size; + local.len = context_size + sizeof(local); + vc += sizeof(local);
See my prior comments about void pointer math.
+ if (copy_to_user(ctx, &local, sizeof(local))) + return -EFAULT; + if (context_size > 0 && copy_to_user(vc, context, context_size)) + return -EFAULT;
Should we handle the padding in this function?
+ return 0; +}
-- paul-moore.com