On Mar 13, 2024 Casey Schaufler <[email protected]> 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: [email protected]
> Fixes: a04a1198088a: ("LSM: syscalls for current process attributes")
> Fixes: ad4aff9ec25f: ("LSM: Create lsm_list_modules system call")
> Signed-off-by: Casey Schaufler <[email protected]>
> Reported-and-reviewed-by: Dmitry V. Levin <[email protected]>
> ---
> 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 <[email protected]> wrote:
> On Mar 13, 2024 Casey Schaufler <[email protected]> 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: [email protected]
> > Fixes: a04a1198088a: ("LSM: syscalls for current process attributes")
> > Fixes: ad4aff9ec25f: ("LSM: Create lsm_list_modules system call")
> > Signed-off-by: Casey Schaufler <[email protected]>
> > Reported-and-reviewed-by: Dmitry V. Levin <[email protected]>
> > ---
> > 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
--
paul-moore.com
On 3/13/2024 3:37 PM, Paul Moore wrote:
> On Wed, Mar 13, 2024 at 4:07 PM Paul Moore <[email protected]> wrote:
>> On Mar 13, 2024 Casey Schaufler <[email protected]> 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: [email protected]
>>> Fixes: a04a1198088a: ("LSM: syscalls for current process attributes")
>>> Fixes: ad4aff9ec25f: ("LSM: Create lsm_list_modules system call")
>>> Signed-off-by: Casey Schaufler <[email protected]>
>>> Reported-and-reviewed-by: Dmitry V. Levin <[email protected]>
>>> ---
>>> 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 4:07 PM Paul Moore <[email protected]> wrote:
> On Mar 13, 2024 Casey Schaufler <[email protected]> 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: [email protected]
> > Fixes: a04a1198088a: ("LSM: syscalls for current process attributes")
> > Fixes: ad4aff9ec25f: ("LSM: Create lsm_list_modules system call")
> > Signed-off-by: Casey Schaufler <[email protected]>
> > Reported-and-reviewed-by: Dmitry V. Levin <[email protected]>
> > ---
> > 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
--
paul-moore.com
On 3/14/2024 8:30 AM, Paul Moore wrote:
> On Wed, Mar 13, 2024 at 4:07 PM Paul Moore <[email protected]> wrote:
>> On Mar 13, 2024 Casey Schaufler <[email protected]> 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: [email protected]
>>> Fixes: a04a1198088a: ("LSM: syscalls for current process attributes")
>>> Fixes: ad4aff9ec25f: ("LSM: Create lsm_list_modules system call")
>>> Signed-off-by: Casey Schaufler <[email protected]>
>>> Reported-and-reviewed-by: Dmitry V. Levin <[email protected]>
>>> ---
>>> 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 <[email protected]> wrote:
> > On Mar 13, 2024 Casey Schaufler <[email protected]> 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: [email protected]
> > > Fixes: a04a1198088a: ("LSM: syscalls for current process attributes")
> > > Fixes: ad4aff9ec25f: ("LSM: Create lsm_list_modules system call")
> > > Signed-off-by: Casey Schaufler <[email protected]>
> > > Reported-and-reviewed-by: Dmitry V. Levin <[email protected]>
> > > ---
> > > 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.
--
ldv
On Thu, Mar 14, 2024 at 2:01 PM Dmitry V. Levin <[email protected]> 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 <[email protected]> wrote:
> > > On Mar 13, 2024 Casey Schaufler <[email protected]> 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: [email protected]
> > > > Fixes: a04a1198088a: ("LSM: syscalls for current process attributes")
> > > > Fixes: ad4aff9ec25f: ("LSM: Create lsm_list_modules system call")
> > > > Signed-off-by: Casey Schaufler <[email protected]>
> > > > Reported-and-reviewed-by: Dmitry V. Levin <[email protected]>
> > > > ---
> > > > 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.
--
paul-moore.com