Messages in this thread | | | Date | Mon, 3 Apr 2023 11:54:57 +0200 | Subject | Re: [PATCH v7 07/11] LSM: Helpers for attribute names and filling an lsm_ctx | From | Mickaël Salaün <> |
| |
On 03/04/2023 11:47, Mickaël Salaün wrote: > > On 15/03/2023 23:47, Casey Schaufler 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(+) > > [...] > >> diff --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); >> + if (copy_to_user(ctx, &local, sizeof(local))) >> + return -EFAULT; >> + if (context_size > 0 && copy_to_user(vc, context, context_size)) >> + return -EFAULT; > > Can we do a single copy_to_user() call? That would avoid inconsistent > user space data, could speed up a bit the operation, and make the code > easier to understand. To use the stack, we need to know the maximum size > of context_size for all use cases, which seems reasonable and can be > checked at build time (on each LSM side, and potentially with specific > context type passed as enum instead of context_size) and run time (for > this generic helper).
Well, actually the context_size should be inferred from id, and the "local" size should be defined and check at build time against all context ID sizes.
> > >> + return 0; >> +} >> + >> /* >> * The default value of the LSM hook is defined in linux/lsm_hook_defs.h and >> * can be accessed with:
| |