[PATCH] security: fix the default value of secid_to_secctx hook

[PATCH] security: fix the default value of secid_to_secctx hook

[Anders Roxell]

security_secid_to_secctx is called by the bpf_lsm hook and a successful
return value (i.e 0) implies that the parameter will be consumed by the
LSM framework. The current behaviour return success when the pointer
isn't initialized when CONFIG_BPF_LSM is enabled, with the default
return from kernel/bpf/bpf_lsm.c.

This is the internal error:

[ 1229.341488][ T2659] usercopy: Kernel memory exposure attempt detected from null address (offset 0, size 280)!
[ 1229.374977][ T2659] ------------[ cut here ]------------
[ 1229.376813][ T2659] kernel BUG at mm/usercopy.c:99!
[ 1229.378398][ T2659] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
[ 1229.380348][ T2659] Modules linked in:
[ 1229.381654][ T2659] CPU: 0 PID: 2659 Comm: systemd-journal Tainted: G    B   W         5.7.0-rc5-next-20200511-00019-g864e0c6319b8-dirty #13
[ 1229.385429][ T2659] Hardware name: linux,dummy-virt (DT)
[ 1229.387143][ T2659] pstate: 80400005 (Nzcv daif +PAN -UAO BTYPE=--)
[ 1229.389165][ T2659] pc : usercopy_abort+0xc8/0xcc
[ 1229.390705][ T2659] lr : usercopy_abort+0xc8/0xcc
[ 1229.392225][ T2659] sp : ffff000064247450
[ 1229.393533][ T2659] x29: ffff000064247460 x28: 0000000000000000
[ 1229.395449][ T2659] x27: 0000000000000118 x26: 0000000000000000
[ 1229.397384][ T2659] x25: ffffa000127049e0 x24: ffffa000127049e0
[ 1229.399306][ T2659] x23: ffffa000127048e0 x22: ffffa000127048a0
[ 1229.401241][ T2659] x21: ffffa00012704b80 x20: ffffa000127049e0
[ 1229.403163][ T2659] x19: ffffa00012704820 x18: 0000000000000000
[ 1229.405094][ T2659] x17: 0000000000000000 x16: 0000000000000000
[ 1229.407008][ T2659] x15: 0000000000000000 x14: 003d090000000000
[ 1229.408942][ T2659] x13: ffff80000d5b25b2 x12: 1fffe0000d5b25b1
[ 1229.410859][ T2659] x11: 1fffe0000d5b25b1 x10: ffff80000d5b25b1
[ 1229.412791][ T2659] x9 : ffffa0001034bee0 x8 : ffff00006ad92d8f
[ 1229.414707][ T2659] x7 : 0000000000000000 x6 : ffffa00015eacb20
[ 1229.416642][ T2659] x5 : ffff0000693c8040 x4 : 0000000000000000
[ 1229.418558][ T2659] x3 : ffffa0001034befc x2 : d57a7483a01c6300
[ 1229.420610][ T2659] x1 : 0000000000000000 x0 : 0000000000000059
[ 1229.422526][ T2659] Call trace:
[ 1229.423631][ T2659]  usercopy_abort+0xc8/0xcc
[ 1229.425091][ T2659]  __check_object_size+0xdc/0x7d4
[ 1229.426729][ T2659]  put_cmsg+0xa30/0xa90
[ 1229.428132][ T2659]  unix_dgram_recvmsg+0x80c/0x930
[ 1229.429731][ T2659]  sock_recvmsg+0x9c/0xc0
[ 1229.431123][ T2659]  ____sys_recvmsg+0x1cc/0x5f8
[ 1229.432663][ T2659]  ___sys_recvmsg+0x100/0x160
[ 1229.434151][ T2659]  __sys_recvmsg+0x110/0x1a8
[ 1229.435623][ T2659]  __arm64_sys_recvmsg+0x58/0x70
[ 1229.437218][ T2659]  el0_svc_common.constprop.1+0x29c/0x340
[ 1229.438994][ T2659]  do_el0_svc+0xe8/0x108
[ 1229.440587][ T2659]  el0_svc+0x74/0x88
[ 1229.441917][ T2659]  el0_sync_handler+0xe4/0x8b4
[ 1229.443464][ T2659]  el0_sync+0x17c/0x180
[ 1229.444920][ T2659] Code: aa1703e2 aa1603e1 910a8260 97ecc860 (d4210000)
[ 1229.447070][ T2659] ---[ end trace 400497d91baeaf51 ]---
[ 1229.448791][ T2659] Kernel panic - not syncing: Fatal exception
[ 1229.450692][ T2659] Kernel Offset: disabled
[ 1229.452061][ T2659] CPU features: 0x240002,20002004
[ 1229.453647][ T2659] Memory Limit: none
[ 1229.455015][ T2659] ---[ end Kernel panic - not syncing: Fatal exception ]---

Rework the so the default return value is -EOPNOTSUPP.

There are likely other callbacks such as security_inode_getsecctx() that
may have the same problem, and that someone that understand the code
better needs to audit them.

Thank you Arnd for helping me figure out what went wrong.

CC: Arnd Bergmann <arnd@arndb.de>
Fixes: 98e828a0650f ("security: Refactor declaration of LSM hooks")
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
 include/linux/lsm_hook_defs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index b9e73d736e13..31eb3381e54b 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -243,7 +243,7 @@ LSM_HOOK(int, -EINVAL, getprocattr, struct task_struct *p, char *name,
 	 char **value)
 LSM_HOOK(int, -EINVAL, setprocattr, const char *name, void *value, size_t size)
 LSM_HOOK(int, 0, ismaclabel, const char *name)
-LSM_HOOK(int, 0, secid_to_secctx, u32 secid, char **secdata,
+LSM_HOOK(int, -EOPNOTSUPP, secid_to_secctx, u32 secid, char **secdata,
 	 u32 *seclen)
 LSM_HOOK(int, 0, secctx_to_secid, const char *secdata, u32 seclen, u32 *secid)
 LSM_HOOK(void, LSM_RET_VOID, release_secctx, char *secdata, u32 seclen)
-- 
2.20.1

Re: [PATCH] security: fix the default value of secid_to_secctx hook

[Alexei Starovoitov]

James,

since you took the previous similar patch are you going to pick this
one up as well?
Or we can route it via bpf tree to Linus asap.

Thanks

On Tue, May 12, 2020 at 10:46 AM Anders Roxell <anders.roxell@linaro.org> wrote:
>
> security_secid_to_secctx is called by the bpf_lsm hook and a successful
> return value (i.e 0) implies that the parameter will be consumed by the
> LSM framework. The current behaviour return success when the pointer
> isn't initialized when CONFIG_BPF_LSM is enabled, with the default
> return from kernel/bpf/bpf_lsm.c.
>
> This is the internal error:
>
> [ 1229.341488][ T2659] usercopy: Kernel memory exposure attempt detected from null address (offset 0, size 280)!
> [ 1229.374977][ T2659] ------------[ cut here ]------------
> [ 1229.376813][ T2659] kernel BUG at mm/usercopy.c:99!
> [ 1229.378398][ T2659] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> [ 1229.380348][ T2659] Modules linked in:
> [ 1229.381654][ T2659] CPU: 0 PID: 2659 Comm: systemd-journal Tainted: G    B   W         5.7.0-rc5-next-20200511-00019-g864e0c6319b8-dirty #13
> [ 1229.385429][ T2659] Hardware name: linux,dummy-virt (DT)
> [ 1229.387143][ T2659] pstate: 80400005 (Nzcv daif +PAN -UAO BTYPE=--)
> [ 1229.389165][ T2659] pc : usercopy_abort+0xc8/0xcc
> [ 1229.390705][ T2659] lr : usercopy_abort+0xc8/0xcc
> [ 1229.392225][ T2659] sp : ffff000064247450
> [ 1229.393533][ T2659] x29: ffff000064247460 x28: 0000000000000000
> [ 1229.395449][ T2659] x27: 0000000000000118 x26: 0000000000000000
> [ 1229.397384][ T2659] x25: ffffa000127049e0 x24: ffffa000127049e0
> [ 1229.399306][ T2659] x23: ffffa000127048e0 x22: ffffa000127048a0
> [ 1229.401241][ T2659] x21: ffffa00012704b80 x20: ffffa000127049e0
> [ 1229.403163][ T2659] x19: ffffa00012704820 x18: 0000000000000000
> [ 1229.405094][ T2659] x17: 0000000000000000 x16: 0000000000000000
> [ 1229.407008][ T2659] x15: 0000000000000000 x14: 003d090000000000
> [ 1229.408942][ T2659] x13: ffff80000d5b25b2 x12: 1fffe0000d5b25b1
> [ 1229.410859][ T2659] x11: 1fffe0000d5b25b1 x10: ffff80000d5b25b1
> [ 1229.412791][ T2659] x9 : ffffa0001034bee0 x8 : ffff00006ad92d8f
> [ 1229.414707][ T2659] x7 : 0000000000000000 x6 : ffffa00015eacb20
> [ 1229.416642][ T2659] x5 : ffff0000693c8040 x4 : 0000000000000000
> [ 1229.418558][ T2659] x3 : ffffa0001034befc x2 : d57a7483a01c6300
> [ 1229.420610][ T2659] x1 : 0000000000000000 x0 : 0000000000000059
> [ 1229.422526][ T2659] Call trace:
> [ 1229.423631][ T2659]  usercopy_abort+0xc8/0xcc
> [ 1229.425091][ T2659]  __check_object_size+0xdc/0x7d4
> [ 1229.426729][ T2659]  put_cmsg+0xa30/0xa90
> [ 1229.428132][ T2659]  unix_dgram_recvmsg+0x80c/0x930
> [ 1229.429731][ T2659]  sock_recvmsg+0x9c/0xc0
> [ 1229.431123][ T2659]  ____sys_recvmsg+0x1cc/0x5f8
> [ 1229.432663][ T2659]  ___sys_recvmsg+0x100/0x160
> [ 1229.434151][ T2659]  __sys_recvmsg+0x110/0x1a8
> [ 1229.435623][ T2659]  __arm64_sys_recvmsg+0x58/0x70
> [ 1229.437218][ T2659]  el0_svc_common.constprop.1+0x29c/0x340
> [ 1229.438994][ T2659]  do_el0_svc+0xe8/0x108
> [ 1229.440587][ T2659]  el0_svc+0x74/0x88
> [ 1229.441917][ T2659]  el0_sync_handler+0xe4/0x8b4
> [ 1229.443464][ T2659]  el0_sync+0x17c/0x180
> [ 1229.444920][ T2659] Code: aa1703e2 aa1603e1 910a8260 97ecc860 (d4210000)
> [ 1229.447070][ T2659] ---[ end trace 400497d91baeaf51 ]---
> [ 1229.448791][ T2659] Kernel panic - not syncing: Fatal exception
> [ 1229.450692][ T2659] Kernel Offset: disabled
> [ 1229.452061][ T2659] CPU features: 0x240002,20002004
> [ 1229.453647][ T2659] Memory Limit: none
> [ 1229.455015][ T2659] ---[ end Kernel panic - not syncing: Fatal exception ]---
>
> Rework the so the default return value is -EOPNOTSUPP.
>
> There are likely other callbacks such as security_inode_getsecctx() that
> may have the same problem, and that someone that understand the code
> better needs to audit them.
>
> Thank you Arnd for helping me figure out what went wrong.
>
> CC: Arnd Bergmann <arnd@arndb.de>
> Fixes: 98e828a0650f ("security: Refactor declaration of LSM hooks")
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> ---
>  include/linux/lsm_hook_defs.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index b9e73d736e13..31eb3381e54b 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -243,7 +243,7 @@ LSM_HOOK(int, -EINVAL, getprocattr, struct task_struct *p, char *name,
>          char **value)
>  LSM_HOOK(int, -EINVAL, setprocattr, const char *name, void *value, size_t size)
>  LSM_HOOK(int, 0, ismaclabel, const char *name)
> -LSM_HOOK(int, 0, secid_to_secctx, u32 secid, char **secdata,
> +LSM_HOOK(int, -EOPNOTSUPP, secid_to_secctx, u32 secid, char **secdata,
>          u32 *seclen)
>  LSM_HOOK(int, 0, secctx_to_secid, const char *secdata, u32 seclen, u32 *secid)
>  LSM_HOOK(void, LSM_RET_VOID, release_secctx, char *secdata, u32 seclen)
> --
> 2.20.1
>

Re: [PATCH] security: fix the default value of secid_to_secctx hook

[James Morris]

On Tue, 12 May 2020, Anders Roxell wrote:

> security_secid_to_secctx is called by the bpf_lsm hook and a successful
> return value (i.e 0) implies that the parameter will be consumed by the
> LSM framework. The current behaviour return success when the pointer
> isn't initialized when CONFIG_BPF_LSM is enabled, with the default
> return from kernel/bpf/bpf_lsm.c.
> 
> This is the internal error:
> 
> [ 1229.341488][ T2659] usercopy: Kernel memory exposure attempt detected from null address (offset 0, size 280)!
> [ 1229.374977][ T2659] ------------[ cut here ]------------
> [ 1229.376813][ T2659] kernel BUG at mm/usercopy.c:99!
> [ 1229.378398][ T2659] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> [ 1229.380348][ T2659] Modules linked in:
> [ 1229.381654][ T2659] CPU: 0 PID: 2659 Comm: systemd-journal Tainted: G    B   W         5.7.0-rc5-next-20200511-00019-g864e0c6319b8-dirty #13
> [ 1229.385429][ T2659] Hardware name: linux,dummy-virt (DT)
> [ 1229.387143][ T2659] pstate: 80400005 (Nzcv daif +PAN -UAO BTYPE=--)
> [ 1229.389165][ T2659] pc : usercopy_abort+0xc8/0xcc
> [ 1229.390705][ T2659] lr : usercopy_abort+0xc8/0xcc
> [ 1229.392225][ T2659] sp : ffff000064247450
> [ 1229.393533][ T2659] x29: ffff000064247460 x28: 0000000000000000
> [ 1229.395449][ T2659] x27: 0000000000000118 x26: 0000000000000000
> [ 1229.397384][ T2659] x25: ffffa000127049e0 x24: ffffa000127049e0
> [ 1229.399306][ T2659] x23: ffffa000127048e0 x22: ffffa000127048a0
> [ 1229.401241][ T2659] x21: ffffa00012704b80 x20: ffffa000127049e0
> [ 1229.403163][ T2659] x19: ffffa00012704820 x18: 0000000000000000
> [ 1229.405094][ T2659] x17: 0000000000000000 x16: 0000000000000000
> [ 1229.407008][ T2659] x15: 0000000000000000 x14: 003d090000000000
> [ 1229.408942][ T2659] x13: ffff80000d5b25b2 x12: 1fffe0000d5b25b1
> [ 1229.410859][ T2659] x11: 1fffe0000d5b25b1 x10: ffff80000d5b25b1
> [ 1229.412791][ T2659] x9 : ffffa0001034bee0 x8 : ffff00006ad92d8f
> [ 1229.414707][ T2659] x7 : 0000000000000000 x6 : ffffa00015eacb20
> [ 1229.416642][ T2659] x5 : ffff0000693c8040 x4 : 0000000000000000
> [ 1229.418558][ T2659] x3 : ffffa0001034befc x2 : d57a7483a01c6300
> [ 1229.420610][ T2659] x1 : 0000000000000000 x0 : 0000000000000059
> [ 1229.422526][ T2659] Call trace:
> [ 1229.423631][ T2659]  usercopy_abort+0xc8/0xcc
> [ 1229.425091][ T2659]  __check_object_size+0xdc/0x7d4
> [ 1229.426729][ T2659]  put_cmsg+0xa30/0xa90
> [ 1229.428132][ T2659]  unix_dgram_recvmsg+0x80c/0x930
> [ 1229.429731][ T2659]  sock_recvmsg+0x9c/0xc0
> [ 1229.431123][ T2659]  ____sys_recvmsg+0x1cc/0x5f8
> [ 1229.432663][ T2659]  ___sys_recvmsg+0x100/0x160
> [ 1229.434151][ T2659]  __sys_recvmsg+0x110/0x1a8
> [ 1229.435623][ T2659]  __arm64_sys_recvmsg+0x58/0x70
> [ 1229.437218][ T2659]  el0_svc_common.constprop.1+0x29c/0x340
> [ 1229.438994][ T2659]  do_el0_svc+0xe8/0x108
> [ 1229.440587][ T2659]  el0_svc+0x74/0x88
> [ 1229.441917][ T2659]  el0_sync_handler+0xe4/0x8b4
> [ 1229.443464][ T2659]  el0_sync+0x17c/0x180
> [ 1229.444920][ T2659] Code: aa1703e2 aa1603e1 910a8260 97ecc860 (d4210000)
> [ 1229.447070][ T2659] ---[ end trace 400497d91baeaf51 ]---
> [ 1229.448791][ T2659] Kernel panic - not syncing: Fatal exception
> [ 1229.450692][ T2659] Kernel Offset: disabled
> [ 1229.452061][ T2659] CPU features: 0x240002,20002004
> [ 1229.453647][ T2659] Memory Limit: none
> [ 1229.455015][ T2659] ---[ end Kernel panic - not syncing: Fatal exception ]---
> 
> Rework the so the default return value is -EOPNOTSUPP.
> 
> There are likely other callbacks such as security_inode_getsecctx() that
> may have the same problem, and that someone that understand the code
> better needs to audit them.
> 
> Thank you Arnd for helping me figure out what went wrong.
> 
> CC: Arnd Bergmann <arnd@arndb.de>
> Fixes: 98e828a0650f ("security: Refactor declaration of LSM hooks")
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>

Note, this patch should have been sent to me and cc'd the LSM list.


Acked-by: James Morris <jamorris@linux.microsoft.com>



-- 
James Morris
<jmorris@namei.org>

Re: [PATCH] security: fix the default value of secid_to_secctx hook

[James Morris]

On Wed, 13 May 2020, Alexei Starovoitov wrote:

> James,
> 
> since you took the previous similar patch are you going to pick this
> one up as well?
> Or we can route it via bpf tree to Linus asap.

Routing via your tree is fine.

> 
> Thanks
> 
> On Tue, May 12, 2020 at 10:46 AM Anders Roxell <anders.roxell@linaro.org> wrote:
> >
> > security_secid_to_secctx is called by the bpf_lsm hook and a successful
> > return value (i.e 0) implies that the parameter will be consumed by the
> > LSM framework. The current behaviour return success when the pointer
> > isn't initialized when CONFIG_BPF_LSM is enabled, with the default
> > return from kernel/bpf/bpf_lsm.c.
> >
> > This is the internal error:
> >
> > [ 1229.341488][ T2659] usercopy: Kernel memory exposure attempt detected from null address (offset 0, size 280)!
> > [ 1229.374977][ T2659] ------------[ cut here ]------------
> > [ 1229.376813][ T2659] kernel BUG at mm/usercopy.c:99!
> > [ 1229.378398][ T2659] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> > [ 1229.380348][ T2659] Modules linked in:
> > [ 1229.381654][ T2659] CPU: 0 PID: 2659 Comm: systemd-journal Tainted: G    B   W         5.7.0-rc5-next-20200511-00019-g864e0c6319b8-dirty #13
> > [ 1229.385429][ T2659] Hardware name: linux,dummy-virt (DT)
> > [ 1229.387143][ T2659] pstate: 80400005 (Nzcv daif +PAN -UAO BTYPE=--)
> > [ 1229.389165][ T2659] pc : usercopy_abort+0xc8/0xcc
> > [ 1229.390705][ T2659] lr : usercopy_abort+0xc8/0xcc
> > [ 1229.392225][ T2659] sp : ffff000064247450
> > [ 1229.393533][ T2659] x29: ffff000064247460 x28: 0000000000000000
> > [ 1229.395449][ T2659] x27: 0000000000000118 x26: 0000000000000000
> > [ 1229.397384][ T2659] x25: ffffa000127049e0 x24: ffffa000127049e0
> > [ 1229.399306][ T2659] x23: ffffa000127048e0 x22: ffffa000127048a0
> > [ 1229.401241][ T2659] x21: ffffa00012704b80 x20: ffffa000127049e0
> > [ 1229.403163][ T2659] x19: ffffa00012704820 x18: 0000000000000000
> > [ 1229.405094][ T2659] x17: 0000000000000000 x16: 0000000000000000
> > [ 1229.407008][ T2659] x15: 0000000000000000 x14: 003d090000000000
> > [ 1229.408942][ T2659] x13: ffff80000d5b25b2 x12: 1fffe0000d5b25b1
> > [ 1229.410859][ T2659] x11: 1fffe0000d5b25b1 x10: ffff80000d5b25b1
> > [ 1229.412791][ T2659] x9 : ffffa0001034bee0 x8 : ffff00006ad92d8f
> > [ 1229.414707][ T2659] x7 : 0000000000000000 x6 : ffffa00015eacb20
> > [ 1229.416642][ T2659] x5 : ffff0000693c8040 x4 : 0000000000000000
> > [ 1229.418558][ T2659] x3 : ffffa0001034befc x2 : d57a7483a01c6300
> > [ 1229.420610][ T2659] x1 : 0000000000000000 x0 : 0000000000000059
> > [ 1229.422526][ T2659] Call trace:
> > [ 1229.423631][ T2659]  usercopy_abort+0xc8/0xcc
> > [ 1229.425091][ T2659]  __check_object_size+0xdc/0x7d4
> > [ 1229.426729][ T2659]  put_cmsg+0xa30/0xa90
> > [ 1229.428132][ T2659]  unix_dgram_recvmsg+0x80c/0x930
> > [ 1229.429731][ T2659]  sock_recvmsg+0x9c/0xc0
> > [ 1229.431123][ T2659]  ____sys_recvmsg+0x1cc/0x5f8
> > [ 1229.432663][ T2659]  ___sys_recvmsg+0x100/0x160
> > [ 1229.434151][ T2659]  __sys_recvmsg+0x110/0x1a8
> > [ 1229.435623][ T2659]  __arm64_sys_recvmsg+0x58/0x70
> > [ 1229.437218][ T2659]  el0_svc_common.constprop.1+0x29c/0x340
> > [ 1229.438994][ T2659]  do_el0_svc+0xe8/0x108
> > [ 1229.440587][ T2659]  el0_svc+0x74/0x88
> > [ 1229.441917][ T2659]  el0_sync_handler+0xe4/0x8b4
> > [ 1229.443464][ T2659]  el0_sync+0x17c/0x180
> > [ 1229.444920][ T2659] Code: aa1703e2 aa1603e1 910a8260 97ecc860 (d4210000)
> > [ 1229.447070][ T2659] ---[ end trace 400497d91baeaf51 ]---
> > [ 1229.448791][ T2659] Kernel panic - not syncing: Fatal exception
> > [ 1229.450692][ T2659] Kernel Offset: disabled
> > [ 1229.452061][ T2659] CPU features: 0x240002,20002004
> > [ 1229.453647][ T2659] Memory Limit: none
> > [ 1229.455015][ T2659] ---[ end Kernel panic - not syncing: Fatal exception ]---
> >
> > Rework the so the default return value is -EOPNOTSUPP.
> >
> > There are likely other callbacks such as security_inode_getsecctx() that
> > may have the same problem, and that someone that understand the code
> > better needs to audit them.
> >
> > Thank you Arnd for helping me figure out what went wrong.
> >
> > CC: Arnd Bergmann <arnd@arndb.de>
> > Fixes: 98e828a0650f ("security: Refactor declaration of LSM hooks")
> > Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> > ---
> >  include/linux/lsm_hook_defs.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> > index b9e73d736e13..31eb3381e54b 100644
> > --- a/include/linux/lsm_hook_defs.h
> > +++ b/include/linux/lsm_hook_defs.h
> > @@ -243,7 +243,7 @@ LSM_HOOK(int, -EINVAL, getprocattr, struct task_struct *p, char *name,
> >          char **value)
> >  LSM_HOOK(int, -EINVAL, setprocattr, const char *name, void *value, size_t size)
> >  LSM_HOOK(int, 0, ismaclabel, const char *name)
> > -LSM_HOOK(int, 0, secid_to_secctx, u32 secid, char **secdata,
> > +LSM_HOOK(int, -EOPNOTSUPP, secid_to_secctx, u32 secid, char **secdata,
> >          u32 *seclen)
> >  LSM_HOOK(int, 0, secctx_to_secid, const char *secdata, u32 seclen, u32 *secid)
> >  LSM_HOOK(void, LSM_RET_VOID, release_secctx, char *secdata, u32 seclen)
> > --
> > 2.20.1
> >
> 


-- 
James Morris
<jamorris@linux.microsoft.com>

Re: [PATCH] security: fix the default value of secid_to_secctx hook

[Alexei Starovoitov]

On Thu, May 14, 2020 at 12:43 PM James Morris
<jamorris@linux.microsoft.com> wrote:
>
> On Wed, 13 May 2020, Alexei Starovoitov wrote:
>
> > James,
> >
> > since you took the previous similar patch are you going to pick this
> > one up as well?
> > Or we can route it via bpf tree to Linus asap.
>
> Routing via your tree is fine.

Perfect.
Applied to bpf tree. Thanks everyone.

Re: [PATCH] security: fix the default value of secid_to_secctx hook

[Alexei Starovoitov]

On Thu, May 14, 2020 at 12:47 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Thu, May 14, 2020 at 12:43 PM James Morris
> <jamorris@linux.microsoft.com> wrote:
> >
> > On Wed, 13 May 2020, Alexei Starovoitov wrote:
> >
> > > James,
> > >
> > > since you took the previous similar patch are you going to pick this
> > > one up as well?
> > > Or we can route it via bpf tree to Linus asap.
> >
> > Routing via your tree is fine.
>
> Perfect.
> Applied to bpf tree. Thanks everyone.

Looks like it was a wrong fix.
It breaks audit like this:
sudo auditctl -e 0
[   88.400296] audit: error in audit_log_task_context
[   88.400976] audit: error in audit_log_task_context
[   88.401597] audit: type=1305 audit(1589584951.198:89): op=set
audit_enabled=0 old=1 auid=0 ses=1 res=0
[   88.402691] audit: type=1300 audit(1589584951.198:89):
arch=c000003e syscall=44 success=yes exit=52 a0=3 a1=7ffe42a37400
a2=34 a3=0 items=0 ppid=2250 pid=2251 auid=0 uid=0 gid=0 euid=0 suid=0
fsuid=0 egid=0 sgid=0 fsgid=0 tty=ttyS0 se)
[   88.405587] audit: type=1327 audit(1589584951.198:89):
proctitle=617564697463746C002D650030
Error sending enable request (Operation not supported)

when CONFIG_LSM= has "bpf" in it.

KP, Anders,
please take a look.

Re: [PATCH] security: fix the default value of secid_to_secctx hook

[Arnd Bergmann]

On Sat, May 16, 2020 at 1:29 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Thu, May 14, 2020 at 12:47 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Thu, May 14, 2020 at 12:43 PM James Morris
> > <jamorris@linux.microsoft.com> wrote:
> > >
> > > On Wed, 13 May 2020, Alexei Starovoitov wrote:
> > >
> > > > James,
> > > >
> > > > since you took the previous similar patch are you going to pick this
> > > > one up as well?
> > > > Or we can route it via bpf tree to Linus asap.
> > >
> > > Routing via your tree is fine.
> >
> > Perfect.
> > Applied to bpf tree. Thanks everyone.
>
> Looks like it was a wrong fix.
> It breaks audit like this:
> sudo auditctl -e 0
> [   88.400296] audit: error in audit_log_task_context
> [   88.400976] audit: error in audit_log_task_context
> [   88.401597] audit: type=1305 audit(1589584951.198:89): op=set
> audit_enabled=0 old=1 auid=0 ses=1 res=0
> [   88.402691] audit: type=1300 audit(1589584951.198:89):
> arch=c000003e syscall=44 success=yes exit=52 a0=3 a1=7ffe42a37400
> a2=34 a3=0 items=0 ppid=2250 pid=2251 auid=0 uid=0 gid=0 euid=0 suid=0
> fsuid=0 egid=0 sgid=0 fsgid=0 tty=ttyS0 se)
> [   88.405587] audit: type=1327 audit(1589584951.198:89):
> proctitle=617564697463746C002D650030
> Error sending enable request (Operation not supported)
>
> when CONFIG_LSM= has "bpf" in it.

Do you have more than one LSM enabled? It looks like
the problem with security_secid_to_secctx() is now that it
returns an error if any of the LSMs fail and the caller expects
it to succeed if at least one of them sets the secdata pointer.

The problem earlier was that the call succeeded even though
no LSM had set the pointer.

What is the behavior we actually expect from this function if
multiple LSM are loaded?

       Arnd

RE: [PATCH] security: fix the default value of secid_to_secctx hook

[Schaufler, Casey]

> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org <linux-kernel-
> owner@vger.kernel.org> On Behalf Of Arnd Bergmann
> Sent: Saturday, May 16, 2020 1:05 AM
> To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Cc: James Morris <jamorris@linux.microsoft.com>; Anders Roxell
> <anders.roxell@linaro.org>; Alexei Starovoitov <ast@kernel.org>; Daniel
> Borkmann <daniel@iogearbox.net>; LKML <linux-kernel@vger.kernel.org>;
> Network Development <netdev@vger.kernel.org>; bpf
> <bpf@vger.kernel.org>
> Subject: Re: [PATCH] security: fix the default value of secid_to_secctx hook

I would *really* appreciate it if discussions about the LSM infrastructure
where done on the linux-security-module mail list. (added to CC).

> 
> On Sat, May 16, 2020 at 1:29 AM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Thu, May 14, 2020 at 12:47 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Thu, May 14, 2020 at 12:43 PM James Morris
> > > <jamorris@linux.microsoft.com> wrote:
> > > >
> > > > On Wed, 13 May 2020, Alexei Starovoitov wrote:
> > > >
> > > > > James,
> > > > >
> > > > > since you took the previous similar patch are you going to pick this
> > > > > one up as well?
> > > > > Or we can route it via bpf tree to Linus asap.
> > > >
> > > > Routing via your tree is fine.
> > >
> > > Perfect.
> > > Applied to bpf tree. Thanks everyone.
> >
> > Looks like it was a wrong fix.
> > It breaks audit like this:
> > sudo auditctl -e 0
> > [   88.400296] audit: error in audit_log_task_context
> > [   88.400976] audit: error in audit_log_task_context
> > [   88.401597] audit: type=1305 audit(1589584951.198:89): op=set
> > audit_enabled=0 old=1 auid=0 ses=1 res=0
> > [   88.402691] audit: type=1300 audit(1589584951.198:89):
> > arch=c000003e syscall=44 success=yes exit=52 a0=3 a1=7ffe42a37400
> > a2=34 a3=0 items=0 ppid=2250 pid=2251 auid=0 uid=0 gid=0 euid=0 suid=0
> > fsuid=0 egid=0 sgid=0 fsgid=0 tty=ttyS0 se)
> > [   88.405587] audit: type=1327 audit(1589584951.198:89):
> > proctitle=617564697463746C002D650030
> > Error sending enable request (Operation not supported)
> >
> > when CONFIG_LSM= has "bpf" in it.
> 
> Do you have more than one LSM enabled? It looks like
> the problem with security_secid_to_secctx() is now that it
> returns an error if any of the LSMs fail and the caller expects
> it to succeed if at least one of them sets the secdata pointer.
> 
> The problem earlier was that the call succeeded even though
> no LSM had set the pointer.
> 
> What is the behavior we actually expect from this function if
> multiple LSM are loaded?
> 
>        Arnd

Re: [PATCH] security: fix the default value of secid_to_secctx hook

[Casey Schaufler]

On 5/18/2020 2:43 PM, Schaufler, Casey wrote:
>> -----Original Message-----
>> From: linux-kernel-owner@vger.kernel.org <linux-kernel-
>> owner@vger.kernel.org> On Behalf Of Arnd Bergmann
>> Sent: Saturday, May 16, 2020 1:05 AM
>> To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
>> Cc: James Morris <jamorris@linux.microsoft.com>; Anders Roxell
>> <anders.roxell@linaro.org>; Alexei Starovoitov <ast@kernel.org>; Daniel
>> Borkmann <daniel@iogearbox.net>; LKML <linux-kernel@vger.kernel.org>;
>> Network Development <netdev@vger.kernel.org>; bpf
>> <bpf@vger.kernel.org>
>> Subject: Re: [PATCH] security: fix the default value of secid_to_secctx hook
> I would *really* appreciate it if discussions about the LSM infrastructure
> where done on the linux-security-module mail list. (added to CC).
>
>> On Sat, May 16, 2020 at 1:29 AM Alexei Starovoitov
>> <alexei.starovoitov@gmail.com> wrote:
>>> On Thu, May 14, 2020 at 12:47 PM Alexei Starovoitov
>>> <alexei.starovoitov@gmail.com> wrote:
>>>> On Thu, May 14, 2020 at 12:43 PM James Morris
>>>> <jamorris@linux.microsoft.com> wrote:
>>>>> On Wed, 13 May 2020, Alexei Starovoitov wrote:
>>>>>
>>>>>> James,
>>>>>>
>>>>>> since you took the previous similar patch are you going to pick this
>>>>>> one up as well?
>>>>>> Or we can route it via bpf tree to Linus asap.
>>>>> Routing via your tree is fine.
>>>> Perfect.
>>>> Applied to bpf tree. Thanks everyone.
>>> Looks like it was a wrong fix.
>>> It breaks audit like this:
>>> sudo auditctl -e 0
>>> [   88.400296] audit: error in audit_log_task_context
>>> [   88.400976] audit: error in audit_log_task_context
>>> [   88.401597] audit: type=1305 audit(1589584951.198:89): op=set
>>> audit_enabled=0 old=1 auid=0 ses=1 res=0
>>> [   88.402691] audit: type=1300 audit(1589584951.198:89):
>>> arch=c000003e syscall=44 success=yes exit=52 a0=3 a1=7ffe42a37400
>>> a2=34 a3=0 items=0 ppid=2250 pid=2251 auid=0 uid=0 gid=0 euid=0 suid=0
>>> fsuid=0 egid=0 sgid=0 fsgid=0 tty=ttyS0 se)
>>> [   88.405587] audit: type=1327 audit(1589584951.198:89):
>>> proctitle=617564697463746C002D650030
>>> Error sending enable request (Operation not supported)
>>>
>>> when CONFIG_LSM= has "bpf" in it.
>> Do you have more than one LSM enabled? It looks like
>> the problem with security_secid_to_secctx() is now that it
>> returns an error if any of the LSMs fail and the caller expects
>> it to succeed if at least one of them sets the secdata pointer.

security_secid_to_secctx() is not currently stackable (I'm
looking at 5.7-rc6) even for this simple case. call_int_hook()
does bail-on-fail and will try all hooks registered, looking for
a failure.

You need to replace the call_int_hook() with an explicit
hlist_for_each_entry(), as is done in security_inode_getsecurity().

>>
>> The problem earlier was that the call succeeded even though
>> no LSM had set the pointer.
>>
>> What is the behavior we actually expect from this function if
>> multiple LSM are loaded?
>>
>>        Arnd