| Message ID | da4d181d-16b9-4e0f-a744-ac61702e0b63@schaufler-ca.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | [v3] LSM: use 32 bit compatible data types in LSM syscalls. | expand |
On Wed, Mar 13, 2024 at 12:32:37PM -0700, Casey Schaufler wrote: > LSM: use 32 bit compatible data types in LSM syscalls. > > Change the size parameters in lsm_list_modules(), lsm_set_self_attr() > and lsm_get_self_attr() from size_t to u32. This avoids the need to > have different interfaces for 32 and 64 bit systems. > > Cc: stable@vger.kernel.org > Fixes: a04a1198088a: ("LSM: syscalls for current process attributes") > Fixes: ad4aff9ec25f: ("LSM: Create lsm_list_modules system call") > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> Reported-and-reviewed-by: Dmitry V. Levin <ldv@strace.io>
On Mar 13, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote: > > LSM: use 32 bit compatible data types in LSM syscalls. > > Change the size parameters in lsm_list_modules(), lsm_set_self_attr() > and lsm_get_self_attr() from size_t to u32. This avoids the need to > have different interfaces for 32 and 64 bit systems. > > Cc: stable@vger.kernel.org > Fixes: a04a1198088a: ("LSM: syscalls for current process attributes") > Fixes: ad4aff9ec25f: ("LSM: Create lsm_list_modules system call") > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > Reported-and-reviewed-by: Dmitry V. Levin <ldv@strace.io> > --- > 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 | 12 ++++++------ > 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 | 10 +++++----- > tools/testing/selftests/lsm/lsm_list_modules_test.c | 8 ++++---- > tools/testing/selftests/lsm/lsm_set_self_attr_test.c | 6 +++--- > 11 files changed, 38 insertions(+), 38 deletions(-) Okay, this looks better, I'm going to merge this into lsm/stable-6.9 and put it through the usual automated testing as well as a kselftest run to make sure everything there is still okay. Assuming all goes well and no one raises any objections, I'll likely send this up to Linus tomorrow. Thanks everyone! -- paul-moore.com
On Wed, Mar 13, 2024 at 4:07 PM Paul Moore <paul@paul-moore.com> wrote: > On Mar 13, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote: > > > > LSM: use 32 bit compatible data types in LSM syscalls. > > > > Change the size parameters in lsm_list_modules(), lsm_set_self_attr() > > and lsm_get_self_attr() from size_t to u32. This avoids the need to > > have different interfaces for 32 and 64 bit systems. > > > > Cc: stable@vger.kernel.org > > Fixes: a04a1198088a: ("LSM: syscalls for current process attributes") > > Fixes: ad4aff9ec25f: ("LSM: Create lsm_list_modules system call") > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > > Reported-and-reviewed-by: Dmitry V. Levin <ldv@strace.io> > > --- > > 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 | 12 ++++++------ > > 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 | 10 +++++----- > > tools/testing/selftests/lsm/lsm_list_modules_test.c | 8 ++++---- > > tools/testing/selftests/lsm/lsm_set_self_attr_test.c | 6 +++--- > > 11 files changed, 38 insertions(+), 38 deletions(-) > > Okay, this looks better, I'm going to merge this into lsm/stable-6.9 > and put it through the usual automated testing as well as a kselftest > run to make sure everything there is still okay. Assuming all goes > well and no one raises any objections, I'll likely send this up to > Linus tomorrow. > > Thanks everyone! Unfortunately it looks like we have a kselftest failure (below). I'm pretty sure that this was working at some point, but it's possible I missed it when I ran the selftests previously. I've got to break for a personal appt right now, but I'll dig into this later tonight. # timeout set to 45 # selftests: lsm: lsm_get_self_attr_test # TAP version 13 # 1..6 # # Starting 6 tests from 1 test cases. # # RUN global.size_null_lsm_get_self_attr ... # # OK global.size_null_lsm_get_self_attr # ok 1 global.size_null_lsm_get_self_attr # # RUN global.ctx_null_lsm_get_self_attr ... # # lsm_get_self_attr_test.c:49:ctx_null_lsm_get_self_attr:Expected -1 (-1) != r c (-1) # # ctx_null_lsm_get_self_attr: Test terminated by assertion # # FAIL global.ctx_null_lsm_get_self_attr # not ok 2 global.ctx_null_lsm_get_self_attr # # RUN global.size_too_small_lsm_get_self_attr ... # # OK global.size_too_small_lsm_get_self_attr # ok 3 global.size_too_small_lsm_get_self_attr # # RUN global.flags_zero_lsm_get_self_attr ... # # OK global.flags_zero_lsm_get_self_attr # ok 4 global.flags_zero_lsm_get_self_attr # # RUN global.flags_overset_lsm_get_self_attr ... # # OK global.flags_overset_lsm_get_self_attr # ok 5 global.flags_overset_lsm_get_self_attr # # RUN global.basic_lsm_get_self_attr ... # # OK global.basic_lsm_get_self_attr # ok 6 global.basic_lsm_get_self_attr # # FAILED: 5 / 6 tests passed. # # Totals: pass:5 fail:1 xfail:0 xpass:0 skip:0 error:0 not ok 1 selftests: lsm: lsm_get_self_attr_test # exit=1
On 3/13/2024 3:37 PM, Paul Moore wrote: > On Wed, Mar 13, 2024 at 4:07 PM Paul Moore <paul@paul-moore.com> wrote: >> On Mar 13, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote: >>> LSM: use 32 bit compatible data types in LSM syscalls. >>> >>> Change the size parameters in lsm_list_modules(), lsm_set_self_attr() >>> and lsm_get_self_attr() from size_t to u32. This avoids the need to >>> have different interfaces for 32 and 64 bit systems. >>> >>> Cc: stable@vger.kernel.org >>> Fixes: a04a1198088a: ("LSM: syscalls for current process attributes") >>> Fixes: ad4aff9ec25f: ("LSM: Create lsm_list_modules system call") >>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> >>> Reported-and-reviewed-by: Dmitry V. Levin <ldv@strace.io> >>> --- >>> 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 | 12 ++++++------ >>> 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 | 10 +++++----- >>> tools/testing/selftests/lsm/lsm_list_modules_test.c | 8 ++++---- >>> tools/testing/selftests/lsm/lsm_set_self_attr_test.c | 6 +++--- >>> 11 files changed, 38 insertions(+), 38 deletions(-) >> Okay, this looks better, I'm going to merge this into lsm/stable-6.9 >> and put it through the usual automated testing as well as a kselftest >> run to make sure everything there is still okay. Assuming all goes >> well and no one raises any objections, I'll likely send this up to >> Linus tomorrow. >> >> Thanks everyone! > Unfortunately it looks like we have a kselftest failure (below). I'm > pretty sure that this was working at some point, but it's possible I > missed it when I ran the selftests previously. I've got to break for > a personal appt right now, but I'll dig into this later tonight. In v2: 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; out: This addresses the case where NULL is passed in the call to lsm_get_self_attr() to get the buffer size required. > > # timeout set to 45 > # selftests: lsm: lsm_get_self_attr_test > # TAP version 13 > # 1..6 > # # Starting 6 tests from 1 test cases. > # # RUN global.size_null_lsm_get_self_attr ... > # # OK global.size_null_lsm_get_self_attr > # ok 1 global.size_null_lsm_get_self_attr > # # RUN global.ctx_null_lsm_get_self_attr ... > # # lsm_get_self_attr_test.c:49:ctx_null_lsm_get_self_attr:Expected -1 (-1) != r > c (-1) > # # ctx_null_lsm_get_self_attr: Test terminated by assertion > # # FAIL global.ctx_null_lsm_get_self_attr > # not ok 2 global.ctx_null_lsm_get_self_attr > # # RUN global.size_too_small_lsm_get_self_attr ... > # # OK global.size_too_small_lsm_get_self_attr > # ok 3 global.size_too_small_lsm_get_self_attr > # # RUN global.flags_zero_lsm_get_self_attr ... > # # OK global.flags_zero_lsm_get_self_attr > # ok 4 global.flags_zero_lsm_get_self_attr > # # RUN global.flags_overset_lsm_get_self_attr ... > # # OK global.flags_overset_lsm_get_self_attr > # ok 5 global.flags_overset_lsm_get_self_attr > # # RUN global.basic_lsm_get_self_attr ... > # # OK global.basic_lsm_get_self_attr > # ok 6 global.basic_lsm_get_self_attr > # # FAILED: 5 / 6 tests passed. > # # Totals: pass:5 fail:1 xfail:0 xpass:0 skip:0 error:0 > not ok 1 selftests: lsm: lsm_get_self_attr_test # exit=1 >
On Wed, Mar 13, 2024 at 6:48 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > On 3/13/2024 3:37 PM, Paul Moore wrote: > > On Wed, Mar 13, 2024 at 4:07 PM Paul Moore <paul@paul-moore.com> wrote: > >> On Mar 13, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote: > >>> LSM: use 32 bit compatible data types in LSM syscalls. > >>> > >>> Change the size parameters in lsm_list_modules(), lsm_set_self_attr() > >>> and lsm_get_self_attr() from size_t to u32. This avoids the need to > >>> have different interfaces for 32 and 64 bit systems. > >>> > >>> Cc: stable@vger.kernel.org > >>> Fixes: a04a1198088a: ("LSM: syscalls for current process attributes") > >>> Fixes: ad4aff9ec25f: ("LSM: Create lsm_list_modules system call") > >>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > >>> Reported-and-reviewed-by: Dmitry V. Levin <ldv@strace.io> > >>> --- > >>> 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 | 12 ++++++------ > >>> 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 | 10 +++++----- > >>> tools/testing/selftests/lsm/lsm_list_modules_test.c | 8 ++++---- > >>> tools/testing/selftests/lsm/lsm_set_self_attr_test.c | 6 +++--- > >>> 11 files changed, 38 insertions(+), 38 deletions(-) > >> Okay, this looks better, I'm going to merge this into lsm/stable-6.9 > >> and put it through the usual automated testing as well as a kselftest > >> run to make sure everything there is still okay. Assuming all goes > >> well and no one raises any objections, I'll likely send this up to > >> Linus tomorrow. > >> > >> Thanks everyone! > > > > Unfortunately it looks like we have a kselftest failure (below). I'm > > pretty sure that this was working at some point, but it's possible I > > missed it when I ran the selftests previously. I've got to break for > > a personal appt right now, but I'll dig into this later tonight. > > In v2: > > 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; > > out: > > This addresses the case where NULL is passed in the call to lsm_get_self_attr() > to get the buffer size required. Yeah, thanks. I didn't get a chance to look at the failure before I had to leave, but now that I'm looking at it I agree. It looks like it used to work prior to d7cf3412a9f6c, but I broke things when I consolidated the processing into lsm_fill_user_ctx() - oops :/ I'll start working on the patch right now and post it as soon as it passes testing.
On Wed, Mar 13, 2024 at 9:44 PM Paul Moore <paul@paul-moore.com> wrote: > On Wed, Mar 13, 2024 at 6:48 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > > On 3/13/2024 3:37 PM, Paul Moore wrote: > > > On Wed, Mar 13, 2024 at 4:07 PM Paul Moore <paul@paul-moore.com> wrote: > > >> On Mar 13, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote: > > >>> LSM: use 32 bit compatible data types in LSM syscalls. > > >>> > > >>> Change the size parameters in lsm_list_modules(), lsm_set_self_attr() > > >>> and lsm_get_self_attr() from size_t to u32. This avoids the need to > > >>> have different interfaces for 32 and 64 bit systems. > > >>> > > >>> Cc: stable@vger.kernel.org > > >>> Fixes: a04a1198088a: ("LSM: syscalls for current process attributes") > > >>> Fixes: ad4aff9ec25f: ("LSM: Create lsm_list_modules system call") > > >>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > > >>> Reported-and-reviewed-by: Dmitry V. Levin <ldv@strace.io> > > >>> --- > > >>> 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 | 12 ++++++------ > > >>> 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 | 10 +++++----- > > >>> tools/testing/selftests/lsm/lsm_list_modules_test.c | 8 ++++---- > > >>> tools/testing/selftests/lsm/lsm_set_self_attr_test.c | 6 +++--- > > >>> 11 files changed, 38 insertions(+), 38 deletions(-) > > >> Okay, this looks better, I'm going to merge this into lsm/stable-6.9 > > >> and put it through the usual automated testing as well as a kselftest > > >> run to make sure everything there is still okay. Assuming all goes > > >> well and no one raises any objections, I'll likely send this up to > > >> Linus tomorrow. > > >> > > >> Thanks everyone! > > > > > > Unfortunately it looks like we have a kselftest failure (below). I'm > > > pretty sure that this was working at some point, but it's possible I > > > missed it when I ran the selftests previously. I've got to break for > > > a personal appt right now, but I'll dig into this later tonight. > > > > In v2: > > > > 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; > > > > out: > > > > This addresses the case where NULL is passed in the call to lsm_get_self_attr() > > to get the buffer size required. > > Yeah, thanks. I didn't get a chance to look at the failure before I > had to leave, but now that I'm looking at it I agree. It looks like > it used to work prior to d7cf3412a9f6c, but I broke things when I > consolidated the processing into lsm_fill_user_ctx() - oops :/ > > I'll start working on the patch right now and post it as soon as it > passes testing. The patch posted below passes the kselftests and all my other sanity checks: https://lore.kernel.org/linux-security-module/20240314022202.599471-2-paul@paul-moore.com
On Wed, Mar 13, 2024 at 4:07 PM Paul Moore <paul@paul-moore.com> wrote: > On Mar 13, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote: > > > > LSM: use 32 bit compatible data types in LSM syscalls. > > > > Change the size parameters in lsm_list_modules(), lsm_set_self_attr() > > and lsm_get_self_attr() from size_t to u32. This avoids the need to > > have different interfaces for 32 and 64 bit systems. > > > > Cc: stable@vger.kernel.org > > Fixes: a04a1198088a: ("LSM: syscalls for current process attributes") > > Fixes: ad4aff9ec25f: ("LSM: Create lsm_list_modules system call") > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > > Reported-and-reviewed-by: Dmitry V. Levin <ldv@strace.io> > > --- > > 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 | 12 ++++++------ > > 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 | 10 +++++----- > > tools/testing/selftests/lsm/lsm_list_modules_test.c | 8 ++++---- > > tools/testing/selftests/lsm/lsm_set_self_attr_test.c | 6 +++--- > > 11 files changed, 38 insertions(+), 38 deletions(-) > > Okay, this looks better, I'm going to merge this into lsm/stable-6.9 > and put it through the usual automated testing as well as a kselftest > run to make sure everything there is still okay. Assuming all goes > well and no one raises any objections, I'll likely send this up to > Linus tomorrow. I had to squash the code snippet below into the patch to address a build problem identified by the kernel build robot. I'm going to keep Casey's sign-off and Dmitry's reported-reviewed tag as I feel this change is minor, but if anyone has any objections please let me know soon. [NOTE: cut-n-paste'd into email, likely whitespace damage, but you get the idea] diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h index 77eb9b0e7685..e619ac10cd23 100644 --- a/include/linux/syscalls.h +++ b/include/linux/syscalls.h @@ -960,10 +960,10 @@ asmlinkage long sys_cachestat(unsigned int fd, struct cachestat __user *cstat, unsigned int flags); asmlinkage long sys_map_shadow_stack(unsigned long addr, unsigned long size, un signed int flags); asmlinkage long sys_lsm_get_self_attr(unsigned int attr, struct lsm_ctx *ctx, - size_t *size, __u32 flags); + u32 *size, u32 flags); asmlinkage long sys_lsm_set_self_attr(unsigned int attr, struct lsm_ctx *ctx, - size_t size, __u32 flags); -asmlinkage long sys_lsm_list_modules(u64 *ids, size_t *size, u32 flags); + u32 size, u32 flags); +asmlinkage long sys_lsm_list_modules(u64 *ids, u32 *size, u32 flags); /* * Architecture-specific system calls
On 3/14/2024 8:30 AM, Paul Moore wrote: > On Wed, Mar 13, 2024 at 4:07 PM Paul Moore <paul@paul-moore.com> wrote: >> On Mar 13, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote: >>> LSM: use 32 bit compatible data types in LSM syscalls. >>> >>> Change the size parameters in lsm_list_modules(), lsm_set_self_attr() >>> and lsm_get_self_attr() from size_t to u32. This avoids the need to >>> have different interfaces for 32 and 64 bit systems. >>> >>> Cc: stable@vger.kernel.org >>> Fixes: a04a1198088a: ("LSM: syscalls for current process attributes") >>> Fixes: ad4aff9ec25f: ("LSM: Create lsm_list_modules system call") >>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> >>> Reported-and-reviewed-by: Dmitry V. Levin <ldv@strace.io> >>> --- >>> 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 | 12 ++++++------ >>> 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 | 10 +++++----- >>> tools/testing/selftests/lsm/lsm_list_modules_test.c | 8 ++++---- >>> tools/testing/selftests/lsm/lsm_set_self_attr_test.c | 6 +++--- >>> 11 files changed, 38 insertions(+), 38 deletions(-) >> Okay, this looks better, I'm going to merge this into lsm/stable-6.9 >> and put it through the usual automated testing as well as a kselftest >> run to make sure everything there is still okay. Assuming all goes >> well and no one raises any objections, I'll likely send this up to >> Linus tomorrow. > I had to squash the code snippet below into the patch to address a > build problem identified by the kernel build robot. I'm going to keep > Casey's sign-off and Dmitry's reported-reviewed tag as I feel this > change is minor, but if anyone has any objections please let me know > soon. Looks fine to me. Thank you. > > [NOTE: cut-n-paste'd into email, likely whitespace damage, but you get the idea] > > diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h > index 77eb9b0e7685..e619ac10cd23 100644 > --- a/include/linux/syscalls.h > +++ b/include/linux/syscalls.h > @@ -960,10 +960,10 @@ asmlinkage long sys_cachestat(unsigned int fd, > struct cachestat __user *cstat, unsigned int flags); > asmlinkage long sys_map_shadow_stack(unsigned long addr, unsigned long size, un > signed int flags); > asmlinkage long sys_lsm_get_self_attr(unsigned int attr, struct lsm_ctx *ctx, > - size_t *size, __u32 flags); > + u32 *size, u32 flags); > asmlinkage long sys_lsm_set_self_attr(unsigned int attr, struct lsm_ctx *ctx, > - size_t size, __u32 flags); > -asmlinkage long sys_lsm_list_modules(u64 *ids, size_t *size, u32 flags); > + u32 size, u32 flags); > +asmlinkage long sys_lsm_list_modules(u64 *ids, u32 *size, u32 flags); > > /* > * Architecture-specific system calls >
On Thu, Mar 14, 2024 at 11:30:53AM -0400, Paul Moore wrote: > On Wed, Mar 13, 2024 at 4:07 PM Paul Moore <paul@paul-moore.com> wrote: > > On Mar 13, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote: > > > > > > LSM: use 32 bit compatible data types in LSM syscalls. > > > > > > Change the size parameters in lsm_list_modules(), lsm_set_self_attr() > > > and lsm_get_self_attr() from size_t to u32. This avoids the need to > > > have different interfaces for 32 and 64 bit systems. > > > > > > Cc: stable@vger.kernel.org > > > Fixes: a04a1198088a: ("LSM: syscalls for current process attributes") > > > Fixes: ad4aff9ec25f: ("LSM: Create lsm_list_modules system call") > > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > > > Reported-and-reviewed-by: Dmitry V. Levin <ldv@strace.io> > > > --- > > > 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 | 12 ++++++------ > > > 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 | 10 +++++----- > > > tools/testing/selftests/lsm/lsm_list_modules_test.c | 8 ++++---- > > > tools/testing/selftests/lsm/lsm_set_self_attr_test.c | 6 +++--- > > > 11 files changed, 38 insertions(+), 38 deletions(-) > > > > Okay, this looks better, I'm going to merge this into lsm/stable-6.9 > > and put it through the usual automated testing as well as a kselftest > > run to make sure everything there is still okay. Assuming all goes > > well and no one raises any objections, I'll likely send this up to > > Linus tomorrow. > > I had to squash the code snippet below into the patch to address a > build problem identified by the kernel build robot. I'm going to keep > Casey's sign-off and Dmitry's reported-reviewed tag as I feel this > change is minor, but if anyone has any objections please let me know > soon. > > [NOTE: cut-n-paste'd into email, likely whitespace damage, but you get the idea] > > diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h > index 77eb9b0e7685..e619ac10cd23 100644 > --- a/include/linux/syscalls.h > +++ b/include/linux/syscalls.h > @@ -960,10 +960,10 @@ asmlinkage long sys_cachestat(unsigned int fd, > struct cachestat __user *cstat, unsigned int flags); > asmlinkage long sys_map_shadow_stack(unsigned long addr, unsigned long size, un > signed int flags); > asmlinkage long sys_lsm_get_self_attr(unsigned int attr, struct lsm_ctx *ctx, > - size_t *size, __u32 flags); > + u32 *size, u32 flags); > asmlinkage long sys_lsm_set_self_attr(unsigned int attr, struct lsm_ctx *ctx, > - size_t size, __u32 flags); > -asmlinkage long sys_lsm_list_modules(u64 *ids, size_t *size, u32 flags); > + u32 size, u32 flags); > +asmlinkage long sys_lsm_list_modules(u64 *ids, u32 *size, u32 flags); Fine with me, thanks. btw, with the change above, u32 will become about twice more popular in include/linux/syscalls.h than __u32.
On Thu, Mar 14, 2024 at 2:01 PM Dmitry V. Levin <ldv@strace.io> wrote: > On Thu, Mar 14, 2024 at 11:30:53AM -0400, Paul Moore wrote: > > On Wed, Mar 13, 2024 at 4:07 PM Paul Moore <paul@paul-moore.com> wrote: > > > On Mar 13, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote: > > > > > > > > LSM: use 32 bit compatible data types in LSM syscalls. > > > > > > > > Change the size parameters in lsm_list_modules(), lsm_set_self_attr() > > > > and lsm_get_self_attr() from size_t to u32. This avoids the need to > > > > have different interfaces for 32 and 64 bit systems. > > > > > > > > Cc: stable@vger.kernel.org > > > > Fixes: a04a1198088a: ("LSM: syscalls for current process attributes") > > > > Fixes: ad4aff9ec25f: ("LSM: Create lsm_list_modules system call") > > > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > > > > Reported-and-reviewed-by: Dmitry V. Levin <ldv@strace.io> > > > > --- > > > > 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 | 12 ++++++------ > > > > 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 | 10 +++++----- > > > > tools/testing/selftests/lsm/lsm_list_modules_test.c | 8 ++++---- > > > > tools/testing/selftests/lsm/lsm_set_self_attr_test.c | 6 +++--- > > > > 11 files changed, 38 insertions(+), 38 deletions(-) > > > > > > Okay, this looks better, I'm going to merge this into lsm/stable-6.9 > > > and put it through the usual automated testing as well as a kselftest > > > run to make sure everything there is still okay. Assuming all goes > > > well and no one raises any objections, I'll likely send this up to > > > Linus tomorrow. > > > > I had to squash the code snippet below into the patch to address a > > build problem identified by the kernel build robot. I'm going to keep > > Casey's sign-off and Dmitry's reported-reviewed tag as I feel this > > change is minor, but if anyone has any objections please let me know > > soon. > > > > [NOTE: cut-n-paste'd into email, likely whitespace damage, but you get the idea] > > > > diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h > > index 77eb9b0e7685..e619ac10cd23 100644 > > --- a/include/linux/syscalls.h > > +++ b/include/linux/syscalls.h > > @@ -960,10 +960,10 @@ asmlinkage long sys_cachestat(unsigned int fd, > > struct cachestat __user *cstat, unsigned int flags); > > asmlinkage long sys_map_shadow_stack(unsigned long addr, unsigned long size, un > > signed int flags); > > asmlinkage long sys_lsm_get_self_attr(unsigned int attr, struct lsm_ctx *ctx, > > - size_t *size, __u32 flags); > > + u32 *size, u32 flags); > > asmlinkage long sys_lsm_set_self_attr(unsigned int attr, struct lsm_ctx *ctx, > > - size_t size, __u32 flags); > > -asmlinkage long sys_lsm_list_modules(u64 *ids, size_t *size, u32 flags); > > + u32 size, u32 flags); > > +asmlinkage long sys_lsm_list_modules(u64 *ids, u32 *size, u32 flags); > > Fine with me, thanks. > > btw, with the change above, u32 will become about twice more popular > in include/linux/syscalls.h than __u32. I was looking at that when I was putting the patch together this morning, trying to decide which was the "correct" choice between 'u32' and '__u32' and wasn't able to find a good explanation of which is the "right" option in this file. Ultimately I went with 'u32' as I tend to follow some old guidance of: '__u32' for userspace headers, 'u32' for kernel headers. If it should be the other way, please let me know. I just want to keep it consistent across the LSM syscalls.
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 76458b6d53da..f9b5baf1b5f4 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -265,9 +265,9 @@ LSM_HOOK(int, 0, netlink_send, struct sock *sk, struct sk_buff *skb) LSM_HOOK(void, LSM_RET_VOID, d_instantiate, struct dentry *dentry, struct inode *inode) LSM_HOOK(int, -EOPNOTSUPP, getselfattr, unsigned int attr, - struct lsm_ctx __user *ctx, size_t *size, u32 flags) + struct lsm_ctx __user *ctx, u32 *size, u32 flags) LSM_HOOK(int, -EOPNOTSUPP, setselfattr, unsigned int attr, - struct lsm_ctx *ctx, size_t size, u32 flags) + struct lsm_ctx *ctx, u32 size, u32 flags) LSM_HOOK(int, -EINVAL, getprocattr, struct task_struct *p, const char *name, char **value) LSM_HOOK(int, -EINVAL, setprocattr, const char *name, void *value, size_t size) diff --git a/include/linux/security.h b/include/linux/security.h index d0eb20f90b26..3180d823e023 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -478,9 +478,9 @@ int security_sem_semop(struct kern_ipc_perm *sma, struct sembuf *sops, unsigned nsops, int alter); void security_d_instantiate(struct dentry *dentry, struct inode *inode); int security_getselfattr(unsigned int attr, struct lsm_ctx __user *ctx, - size_t __user *size, u32 flags); + u32 __user *size, u32 flags); int security_setselfattr(unsigned int attr, struct lsm_ctx __user *ctx, - size_t size, u32 flags); + u32 size, u32 flags); int security_getprocattr(struct task_struct *p, int lsmid, const char *name, char **value); int security_setprocattr(int lsmid, const char *name, void *value, size_t size); @@ -494,7 +494,7 @@ int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen); int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen); int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen); int security_locked_down(enum lockdown_reason what); -int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, size_t *uctx_len, +int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, u32 *uctx_len, void *val, size_t val_len, u64 id, u64 flags); #else /* CONFIG_SECURITY */ @@ -1434,7 +1434,7 @@ static inline int security_locked_down(enum lockdown_reason what) return 0; } static inline int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, - size_t *uctx_len, void *val, size_t val_len, + u32 *uctx_len, void *val, size_t val_len, u64 id, u64 flags) { return -EOPNOTSUPP; diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 9a3dcaafb5b1..cef8c466af80 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -779,7 +779,7 @@ static int apparmor_sb_pivotroot(const struct path *old_path, } static int apparmor_getselfattr(unsigned int attr, struct lsm_ctx __user *lx, - size_t *size, u32 flags) + u32 *size, u32 flags) { int error = -ENOENT; struct aa_task_ctx *ctx = task_ctx(current); @@ -924,7 +924,7 @@ static int do_setattr(u64 attr, void *value, size_t size) } static int apparmor_setselfattr(unsigned int attr, struct lsm_ctx *ctx, - size_t size, u32 flags) + u32 size, u32 flags) { int rc; diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c index 5d391b1f7e69..8440948a690c 100644 --- a/security/lsm_syscalls.c +++ b/security/lsm_syscalls.c @@ -53,7 +53,7 @@ u64 lsm_name_to_attr(const char *name) * value indicating the reason for the error is returned. */ SYSCALL_DEFINE4(lsm_set_self_attr, unsigned int, attr, struct lsm_ctx __user *, - ctx, size_t, size, u32, flags) + ctx, u32, size, u32, flags) { return security_setselfattr(attr, ctx, size, flags); } @@ -75,7 +75,7 @@ SYSCALL_DEFINE4(lsm_set_self_attr, unsigned int, attr, struct lsm_ctx __user *, * 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) + ctx, u32 __user *, size, u32, flags) { return security_getselfattr(attr, ctx, size, flags); } @@ -93,11 +93,11 @@ SYSCALL_DEFINE4(lsm_get_self_attr, unsigned int, attr, struct lsm_ctx __user *, * required size. In all other cases a negative value indicating the * error is returned. */ -SYSCALL_DEFINE3(lsm_list_modules, u64 __user *, ids, size_t __user *, size, +SYSCALL_DEFINE3(lsm_list_modules, u64 __user *, ids, u32 __user *, size, u32, flags) { - size_t total_size = lsm_active_cnt * sizeof(*ids); - size_t usize; + u32 total_size = lsm_active_cnt * sizeof(*ids); + u32 usize; int i; if (flags) diff --git a/security/security.c b/security/security.c index 7035ee35a393..fb7505c73485 100644 --- a/security/security.c +++ b/security/security.c @@ -785,7 +785,7 @@ static int lsm_superblock_alloc(struct super_block *sb) * Returns 0 on success, -E2BIG if userspace buffer is not large enough, * -EFAULT on a copyout error, -ENOMEM if memory can't be allocated. */ -int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, size_t *uctx_len, +int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, u32 *uctx_len, void *val, size_t val_len, u64 id, u64 flags) { @@ -3918,14 +3918,14 @@ EXPORT_SYMBOL(security_d_instantiate); * If @size is insufficient to contain the data -E2BIG is returned. */ int security_getselfattr(unsigned int attr, struct lsm_ctx __user *uctx, - size_t __user *size, u32 flags) + u32 __user *size, u32 flags) { struct security_hook_list *hp; struct lsm_ctx lctx = { .id = LSM_ID_UNDEF, }; u8 __user *base = (u8 __user *)uctx; - size_t total = 0; - size_t entrysize; - size_t left; + u32 entrysize; + u32 total = 0; + u32 left; bool toobig = false; bool single = false; int count = 0; @@ -4011,7 +4011,7 @@ int security_getselfattr(unsigned int attr, struct lsm_ctx __user *uctx, * LSM specific failure. */ int security_setselfattr(unsigned int attr, struct lsm_ctx __user *uctx, - size_t size, u32 flags) + u32 size, u32 flags) { struct security_hook_list *hp; struct lsm_ctx *lctx; diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 338b023a8c3e..71e6e7079d7f 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6556,7 +6556,7 @@ static int selinux_lsm_setattr(u64 attr, void *value, size_t size) * There will only ever be one attribute. */ static int selinux_getselfattr(unsigned int attr, struct lsm_ctx __user *ctx, - size_t *size, u32 flags) + u32 *size, u32 flags) { int rc; char *val = NULL; @@ -6571,7 +6571,7 @@ static int selinux_getselfattr(unsigned int attr, struct lsm_ctx __user *ctx, } static int selinux_setselfattr(unsigned int attr, struct lsm_ctx *ctx, - size_t size, u32 flags) + u32 size, u32 flags) { int rc; diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 0fdbf04cc258..0891fcc11e40 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -3641,7 +3641,7 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode) * There will only ever be one attribute. */ static int smack_getselfattr(unsigned int attr, struct lsm_ctx __user *ctx, - size_t *size, u32 flags) + u32 *size, u32 flags) { int rc; struct smack_known *skp; @@ -3762,7 +3762,7 @@ static int do_setattr(u64 attr, void *value, size_t size) * Returns 0 on success, an error code otherwise. */ static int smack_setselfattr(unsigned int attr, struct lsm_ctx *ctx, - size_t size, u32 flags) + u32 size, u32 flags) { int rc; diff --git a/tools/testing/selftests/lsm/common.h b/tools/testing/selftests/lsm/common.h index d404329e5eeb..06d12110d241 100644 --- a/tools/testing/selftests/lsm/common.h +++ b/tools/testing/selftests/lsm/common.h @@ -7,7 +7,7 @@ #ifndef lsm_get_self_attr static inline int lsm_get_self_attr(unsigned int attr, struct lsm_ctx *ctx, - size_t *size, __u32 flags) + __u32 *size, __u32 flags) { return syscall(__NR_lsm_get_self_attr, attr, ctx, size, flags); } @@ -15,14 +15,14 @@ static inline int lsm_get_self_attr(unsigned int attr, struct lsm_ctx *ctx, #ifndef lsm_set_self_attr static inline int lsm_set_self_attr(unsigned int attr, struct lsm_ctx *ctx, - size_t size, __u32 flags) + __u32 size, __u32 flags) { return syscall(__NR_lsm_set_self_attr, attr, ctx, size, flags); } #endif #ifndef lsm_list_modules -static inline int lsm_list_modules(__u64 *ids, size_t *size, __u32 flags) +static inline int lsm_list_modules(__u64 *ids, __u32 *size, __u32 flags) { return syscall(__NR_lsm_list_modules, ids, size, flags); } 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..df215e4aa63f 100644 --- a/tools/testing/selftests/lsm/lsm_get_self_attr_test.c +++ b/tools/testing/selftests/lsm/lsm_get_self_attr_test.c @@ -40,7 +40,7 @@ TEST(size_null_lsm_get_self_attr) TEST(ctx_null_lsm_get_self_attr) { const long page_size = sysconf(_SC_PAGESIZE); - size_t size = page_size; + __u32 size = page_size; int rc; rc = lsm_get_self_attr(LSM_ATTR_CURRENT, NULL, &size, 0); @@ -57,7 +57,7 @@ TEST(size_too_small_lsm_get_self_attr) { const long page_size = sysconf(_SC_PAGESIZE); struct lsm_ctx *ctx = calloc(page_size, 1); - size_t size = 1; + __u32 size = 1; ASSERT_NE(NULL, ctx); errno = 0; @@ -77,7 +77,7 @@ 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 size; int lsmcount; int i; @@ -117,7 +117,7 @@ TEST(flags_overset_lsm_get_self_attr) { const long page_size = sysconf(_SC_PAGESIZE); struct lsm_ctx *ctx = calloc(page_size, 1); - size_t size; + __u32 size; ASSERT_NE(NULL, ctx); @@ -140,7 +140,7 @@ TEST(flags_overset_lsm_get_self_attr) TEST(basic_lsm_get_self_attr) { const long page_size = sysconf(_SC_PAGESIZE); - size_t size = page_size; + __u32 size = page_size; struct lsm_ctx *ctx = calloc(page_size, 1); struct lsm_ctx *tctx = NULL; __u64 *syscall_lsms = calloc(page_size, 1); diff --git a/tools/testing/selftests/lsm/lsm_list_modules_test.c b/tools/testing/selftests/lsm/lsm_list_modules_test.c index 9df29b1e3497..868641dbb309 100644 --- a/tools/testing/selftests/lsm/lsm_list_modules_test.c +++ b/tools/testing/selftests/lsm/lsm_list_modules_test.c @@ -31,7 +31,7 @@ TEST(size_null_lsm_list_modules) TEST(ids_null_lsm_list_modules) { const long page_size = sysconf(_SC_PAGESIZE); - size_t size = page_size; + __u32 size = page_size; errno = 0; ASSERT_EQ(-1, lsm_list_modules(NULL, &size, 0)); @@ -43,7 +43,7 @@ TEST(size_too_small_lsm_list_modules) { const long page_size = sysconf(_SC_PAGESIZE); __u64 *syscall_lsms = calloc(page_size, 1); - size_t size = 1; + __u32 size = 1; ASSERT_NE(NULL, syscall_lsms); errno = 0; @@ -58,7 +58,7 @@ TEST(flags_set_lsm_list_modules) { const long page_size = sysconf(_SC_PAGESIZE); __u64 *syscall_lsms = calloc(page_size, 1); - size_t size = page_size; + __u32 size = page_size; ASSERT_NE(NULL, syscall_lsms); errno = 0; @@ -72,7 +72,7 @@ TEST(flags_set_lsm_list_modules) TEST(correct_lsm_list_modules) { const long page_size = sysconf(_SC_PAGESIZE); - size_t size = page_size; + __u32 size = page_size; __u64 *syscall_lsms = calloc(page_size, 1); char *sysfs_lsms = calloc(page_size, 1); char *name; diff --git a/tools/testing/selftests/lsm/lsm_set_self_attr_test.c b/tools/testing/selftests/lsm/lsm_set_self_attr_test.c index e9712c6cf596..66dec47e3ca3 100644 --- a/tools/testing/selftests/lsm/lsm_set_self_attr_test.c +++ b/tools/testing/selftests/lsm/lsm_set_self_attr_test.c @@ -25,7 +25,7 @@ TEST(size_too_small_lsm_set_self_attr) { const long page_size = sysconf(_SC_PAGESIZE); struct lsm_ctx *ctx = calloc(page_size, 1); - size_t size = page_size; + __u32 size = page_size; ASSERT_NE(NULL, ctx); if (attr_lsm_count()) { @@ -41,7 +41,7 @@ TEST(flags_zero_lsm_set_self_attr) { const long page_size = sysconf(_SC_PAGESIZE); struct lsm_ctx *ctx = calloc(page_size, 1); - size_t size = page_size; + __u32 size = page_size; ASSERT_NE(NULL, ctx); if (attr_lsm_count()) { @@ -57,7 +57,7 @@ TEST(flags_overset_lsm_set_self_attr) { const long page_size = sysconf(_SC_PAGESIZE); char *ctx = calloc(page_size, 1); - size_t size = page_size; + __u32 size = page_size; struct lsm_ctx *tctx = (struct lsm_ctx *)ctx; ASSERT_NE(NULL, ctx);
LSM: use 32 bit compatible data types in LSM syscalls. Change the size parameters in lsm_list_modules(), lsm_set_self_attr() and lsm_get_self_attr() from size_t to u32. This avoids the need to have different interfaces for 32 and 64 bit systems. Cc: stable@vger.kernel.org Fixes: a04a1198088a: ("LSM: syscalls for current process attributes") Fixes: ad4aff9ec25f: ("LSM: Create lsm_list_modules system call") Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> --- 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 | 12 ++++++------ 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 | 10 +++++----- tools/testing/selftests/lsm/lsm_list_modules_test.c | 8 ++++---- tools/testing/selftests/lsm/lsm_set_self_attr_test.c | 6 +++--- 11 files changed, 38 insertions(+), 38 deletions(-)