Messages in this thread | | | Date | Wed, 13 Mar 2024 14:46:21 -0400 | From | Paul Moore <> | Subject | Re: [PATCH v2] LSM: use 32 bit compatible data types in LSM syscalls. |
| |
On Mar 13, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote: > > LSM: use 32 bit compatible data types in LSM syscalls. > > Change the size paramters in lsm_list_modules(), lsm_set_self_attr()
s/paramters/parameters/
> and lsm_get_self_attr() from size_t to u32. This avoids the need to > have different interfaces for 32 and 64 bit systems. > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
We should add the following 'Fixes:' tags as well as a stable marking:
Cc: stable@vger.kernel.org Fixes: a04a1198088a ("LSM: syscalls for current process attributes") Fixes: ad4aff9ec25f ("LSM: Create lsm_list_modules system call")
> --- > include/linux/lsm_hook_defs.h | 4 ++-- > include/linux/security.h | 8 ++++---- > security/apparmor/lsm.c | 4 ++-- > security/lsm_syscalls.c | 10 +++++----- > security/security.c | 14 +++++++------- > security/selinux/hooks.c | 4 ++-- > security/smack/smack_lsm.c | 4 ++-- > tools/testing/selftests/lsm/common.h | 6 +++--- > tools/testing/selftests/lsm/lsm_get_self_attr_test.c | 12 ++++++------ > tools/testing/selftests/lsm/lsm_list_modules_test.c | 8 ++++---- > tools/testing/selftests/lsm/lsm_set_self_attr_test.c | 6 +++--- > 11 files changed, 40 insertions(+), 40 deletions(-)
..
> diff --git a/security/security.c b/security/security.c > index 7035ee35a393..a0f9caf89ae1 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -810,7 +810,7 @@ int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, size_t *uctx_len, > nctx->ctx_len = val_len; > memcpy(nctx->ctx, val, val_len); > > - if (copy_to_user(uctx, nctx, nctx_len)) > + if (uctx && copy_to_user(uctx, nctx, nctx_len)) > rc = -EFAULT;
Hey, where did that @uctx check come from?
I'm trying to work through if that is a good/bad change, but regardless of if we want to make that change, it really should be in a separate patch as it has nothing to do with the syscall parameter changes.
> diff --git a/tools/testing/selftests/lsm/lsm_get_self_attr_test.c b/tools/testing/selftests/lsm/lsm_get_self_attr_test.c > index e0e313d9047a..288302a444e0 100644 > --- a/tools/testing/selftests/lsm/lsm_get_self_attr_test.c > +++ b/tools/testing/selftests/lsm/lsm_get_self_attr_test.c > @@ -76,8 +76,8 @@ TEST(flags_zero_lsm_get_self_attr) > { > const long page_size = sysconf(_SC_PAGESIZE); > struct lsm_ctx *ctx = calloc(page_size, 1); > - __u64 *syscall_lsms = calloc(page_size, 1); > - size_t size; > + __u32 *syscall_lsms = calloc(page_size, 1);
I believe that should remain a __u64 pointer as we didn't change the first parameter to lsm_list_modules(). I'm guessing this was an victim of an overzealous /u64/u32/ search-n-replace going from v1 to v2.
> + __u32 size; > int lsmcount; > int i; >
In the interest of speeding things along, I'm happy to make the above changes while merging Casey, but if you would prefer to do a respin that's fine with me - let me know either way so I can plan accordingly.
-- paul-moore.com
| |