2020-01-08 08:53:02

by Huaisheng Ye

[permalink] [raw]
Subject: [PATCH] LSM: Delete hooks in reverse order for avoiding race

From: Huaisheng Ye <[email protected]>

There is small possibility as race condition when selinux_disable
has been triggered. security_delete_hooks deletes all selinux hooks
from security_hook_heads, but there are some selinux functions which
are being called at the same time.

Here is a panic accident scene from 4.18 based kernel,

[ 26.654494] SELinux: Disabled at runtime.
[ 26.654507] BUG: unable to handle kernel NULL pointer dereference
at 0000000000000020
[ 26.654508] PGD 0 P4D 0
[ 26.654510] Oops: 0002 [#1] SMP NOPTI
[ 26.654512] CPU: 53 PID: 2614 Comm: systemd-cgroups Tainted: G
OE --------- - - 4.18.0-80.el8.x86_64 #1
[ 26.654512] Hardware name: Lenovo ThinkSystem SR850P
-[7D2H]-/-[7D2H]-, BIOS -[TEE145P-1.10]- 12/06/2019
[ 26.654519] RIP: 0010:selinux_socket_post_create+0x80/0x390
[ 26.654520] Code: e9 95 6a 89 00 bd 16 00 00 00 c7 44 24 04 01
00 00 00 45 85 c0 0f 85 f6 00 00 00 8b 56 14 85 d2 0f 84 26 01 00
00 89 54 24 04 <66> 41 89 6c 24 20 31 c0 41 89 54 24 1c 41 c6 44
24 22 01 49 8b 4d
[ 26.654521] RSP: 0018:ffffbf515cc63e48 EFLAGS: 00010246
[ 26.654522] RAX: 0000000000000000 RBX: 0000000000000001 RCX: 0000000000000019
[ 26.654522] RDX: 0000000000000001 RSI: 0000000000000001 RDI: ffffffffab46f680
[ 26.654523] RBP: 0000000000000019 R08: 0000000000000000 R09: ffffbf515cc63e4c
[ 26.654523] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
[ 26.654524] R13: ffff97d7bb6cbc80 R14: 0000000000000001 R15: ffff97d7bb6cbc80
[ 26.654525] FS: 00007f5c608ea380(0000) GS:ffff97d7bf140000(0000) knlGS:0000000000000000
[ 26.654525] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 26.654526] CR2: 0000000000000020 CR3: 0000011ebc934004 CR4: 00000000007606e0
[ 26.654527] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 26.654528] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 26.654528] PKRU: 55555554
[ 26.654528] Call Trace:
[ 26.654535] security_socket_post_create+0x42/0x60
[ 26.654537] SELinux: Unregistering netfilter hooks
[ 26.654542] __sock_create+0x106/0x1a0
[ 26.654545] __sys_socket+0x57/0xe0
[ 26.654547] __x64_sys_socket+0x16/0x20
[ 26.654551] do_syscall_64+0x5b/0x1b0
[ 26.654554] entry_SYSCALL_64_after_hwframe+0x65/0xca

The root cause is that, selinux_inode_alloc_security has been deleted
firstly from security_hook_heads, so security_inode_alloc directly
return 0, that means the value of pointer inode->i_security equalling
to NULL.

But selinux_socket_post_create hasn't been deleted at that moment, so
which would involked by mistake. Inside the function, pointer isec
needs to point to inode->i_security, then a NULL pointer defect happens.

For current upstream kernel, because of commit
afb1cbe37440c7f38b9cf46fc331cc9dfd5cce21
the inode security has been moved out to LSM infrastructure from
individual security modules like selinux.

But this patch still can be applied for solving similar issue when
security_delete_hooks has been used. Also for stable branch v4.19,
the inode security still need to be created in individual modules.

The patch has been verified by Lenovo SR850P server through overnight
reboot cycles.

Signed-off-by: Huaisheng Ye <[email protected]>
---
include/linux/lsm_hooks.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 20d8cf1..57cb2ac 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2164,7 +2164,7 @@ static inline void security_delete_hooks(struct security_hook_list *hooks,
int i;

for (i = 0; i < count; i++)
- hlist_del_rcu(&hooks[i].list);
+ hlist_del_rcu(&hooks[count - 1 - i].list);
}
#endif /* CONFIG_SECURITY_SELINUX_DISABLE */

--
1.8.3.1



2020-01-08 11:44:25

by Ondrej Mosnacek

[permalink] [raw]
Subject: Re: [PATCH] LSM: Delete hooks in reverse order for avoiding race

Hi,

On Wed, Jan 8, 2020 at 9:51 AM Huaisheng Ye <[email protected]> wrote:
> From: Huaisheng Ye <[email protected]>
>
> There is small possibility as race condition when selinux_disable
> has been triggered. security_delete_hooks deletes all selinux hooks
> from security_hook_heads, but there are some selinux functions which
> are being called at the same time.
>
> Here is a panic accident scene from 4.18 based kernel,
>
> [ 26.654494] SELinux: Disabled at runtime.
> [ 26.654507] BUG: unable to handle kernel NULL pointer dereference
> at 0000000000000020
> [ 26.654508] PGD 0 P4D 0
> [ 26.654510] Oops: 0002 [#1] SMP NOPTI
> [ 26.654512] CPU: 53 PID: 2614 Comm: systemd-cgroups Tainted: G
> OE --------- - - 4.18.0-80.el8.x86_64 #1
> [ 26.654512] Hardware name: Lenovo ThinkSystem SR850P
> -[7D2H]-/-[7D2H]-, BIOS -[TEE145P-1.10]- 12/06/2019
> [ 26.654519] RIP: 0010:selinux_socket_post_create+0x80/0x390
> [ 26.654520] Code: e9 95 6a 89 00 bd 16 00 00 00 c7 44 24 04 01
> 00 00 00 45 85 c0 0f 85 f6 00 00 00 8b 56 14 85 d2 0f 84 26 01 00
> 00 89 54 24 04 <66> 41 89 6c 24 20 31 c0 41 89 54 24 1c 41 c6 44
> 24 22 01 49 8b 4d
> [ 26.654521] RSP: 0018:ffffbf515cc63e48 EFLAGS: 00010246
> [ 26.654522] RAX: 0000000000000000 RBX: 0000000000000001 RCX: 0000000000000019
> [ 26.654522] RDX: 0000000000000001 RSI: 0000000000000001 RDI: ffffffffab46f680
> [ 26.654523] RBP: 0000000000000019 R08: 0000000000000000 R09: ffffbf515cc63e4c
> [ 26.654523] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
> [ 26.654524] R13: ffff97d7bb6cbc80 R14: 0000000000000001 R15: ffff97d7bb6cbc80
> [ 26.654525] FS: 00007f5c608ea380(0000) GS:ffff97d7bf140000(0000) knlGS:0000000000000000
> [ 26.654525] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 26.654526] CR2: 0000000000000020 CR3: 0000011ebc934004 CR4: 00000000007606e0
> [ 26.654527] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 26.654528] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 26.654528] PKRU: 55555554
> [ 26.654528] Call Trace:
> [ 26.654535] security_socket_post_create+0x42/0x60
> [ 26.654537] SELinux: Unregistering netfilter hooks
> [ 26.654542] __sock_create+0x106/0x1a0
> [ 26.654545] __sys_socket+0x57/0xe0
> [ 26.654547] __x64_sys_socket+0x16/0x20
> [ 26.654551] do_syscall_64+0x5b/0x1b0
> [ 26.654554] entry_SYSCALL_64_after_hwframe+0x65/0xca
>
> The root cause is that, selinux_inode_alloc_security has been deleted
> firstly from security_hook_heads, so security_inode_alloc directly
> return 0, that means the value of pointer inode->i_security equalling
> to NULL.
>
> But selinux_socket_post_create hasn't been deleted at that moment, so
> which would involked by mistake. Inside the function, pointer isec
> needs to point to inode->i_security, then a NULL pointer defect happens.
>
> For current upstream kernel, because of commit
> afb1cbe37440c7f38b9cf46fc331cc9dfd5cce21
> the inode security has been moved out to LSM infrastructure from
> individual security modules like selinux.
>
> But this patch still can be applied for solving similar issue when
> security_delete_hooks has been used. Also for stable branch v4.19,
> the inode security still need to be created in individual modules.

Thank you for the patch, however there are already existing proposed
patches to fix this issue, see [1], [2], and [3]. At the moment it
looks like the SELinux hooks reorder approach ([1]) will be accepted
as a temporary solution (the SELinux runtime disable is being
deprecated [4] in favor of properly disabling SELinux by setting
selinux=0 on the kernel command line).

Your approach unfortunately isn't robust (depends on assumptions about
how hooks are ordered by LSMs) nor complete (even the inverse order
still has some race conditions that may lead to a crash - e.g.
selinux_bpf_map() vs. selinux_bpf_map_alloc()).

Also, please, don't forget to Cc the LSM/SELinux mailing lists
([email protected]/[email protected],
respectively) for patches related to the LSM framework/SELinux.

[1] https://lore.kernel.org/selinux/[email protected]/T/
[2] https://lore.kernel.org/selinux/[email protected]/T/
[3] https://lore.kernel.org/selinux/[email protected]/T/
[4] https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git/commit/?h=next&id=89b223bfb8a89731bea4c84982b5d2ad7ba460e3

>
> The patch has been verified by Lenovo SR850P server through overnight
> reboot cycles.
>
> Signed-off-by: Huaisheng Ye <[email protected]>
> ---
> include/linux/lsm_hooks.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 20d8cf1..57cb2ac 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -2164,7 +2164,7 @@ static inline void security_delete_hooks(struct security_hook_list *hooks,
> int i;
>
> for (i = 0; i < count; i++)
> - hlist_del_rcu(&hooks[i].list);
> + hlist_del_rcu(&hooks[count - 1 - i].list);
> }
> #endif /* CONFIG_SECURITY_SELINUX_DISABLE */
>
> --
> 1.8.3.1
>
>

--
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.

2020-01-08 19:49:15

by James Morris

[permalink] [raw]
Subject: Re: [PATCH] LSM: Delete hooks in reverse order for avoiding race

Please cc the LSM list with LSM related patches.


On Wed, 8 Jan 2020, Huaisheng Ye wrote:

> From: Huaisheng Ye <[email protected]>
>
> There is small possibility as race condition when selinux_disable
> has been triggered. security_delete_hooks deletes all selinux hooks
> from security_hook_heads, but there are some selinux functions which
> are being called at the same time.
>
> Here is a panic accident scene from 4.18 based kernel,
>
> [ 26.654494] SELinux: Disabled at runtime.
> [ 26.654507] BUG: unable to handle kernel NULL pointer dereference
> at 0000000000000020
> [ 26.654508] PGD 0 P4D 0
> [ 26.654510] Oops: 0002 [#1] SMP NOPTI
> [ 26.654512] CPU: 53 PID: 2614 Comm: systemd-cgroups Tainted: G
> OE --------- - - 4.18.0-80.el8.x86_64 #1
> [ 26.654512] Hardware name: Lenovo ThinkSystem SR850P
> -[7D2H]-/-[7D2H]-, BIOS -[TEE145P-1.10]- 12/06/2019
> [ 26.654519] RIP: 0010:selinux_socket_post_create+0x80/0x390
> [ 26.654520] Code: e9 95 6a 89 00 bd 16 00 00 00 c7 44 24 04 01
> 00 00 00 45 85 c0 0f 85 f6 00 00 00 8b 56 14 85 d2 0f 84 26 01 00
> 00 89 54 24 04 <66> 41 89 6c 24 20 31 c0 41 89 54 24 1c 41 c6 44
> 24 22 01 49 8b 4d
> [ 26.654521] RSP: 0018:ffffbf515cc63e48 EFLAGS: 00010246
> [ 26.654522] RAX: 0000000000000000 RBX: 0000000000000001 RCX: 0000000000000019
> [ 26.654522] RDX: 0000000000000001 RSI: 0000000000000001 RDI: ffffffffab46f680
> [ 26.654523] RBP: 0000000000000019 R08: 0000000000000000 R09: ffffbf515cc63e4c
> [ 26.654523] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
> [ 26.654524] R13: ffff97d7bb6cbc80 R14: 0000000000000001 R15: ffff97d7bb6cbc80
> [ 26.654525] FS: 00007f5c608ea380(0000) GS:ffff97d7bf140000(0000) knlGS:0000000000000000
> [ 26.654525] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 26.654526] CR2: 0000000000000020 CR3: 0000011ebc934004 CR4: 00000000007606e0
> [ 26.654527] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 26.654528] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 26.654528] PKRU: 55555554
> [ 26.654528] Call Trace:
> [ 26.654535] security_socket_post_create+0x42/0x60
> [ 26.654537] SELinux: Unregistering netfilter hooks
> [ 26.654542] __sock_create+0x106/0x1a0
> [ 26.654545] __sys_socket+0x57/0xe0
> [ 26.654547] __x64_sys_socket+0x16/0x20
> [ 26.654551] do_syscall_64+0x5b/0x1b0
> [ 26.654554] entry_SYSCALL_64_after_hwframe+0x65/0xca
>
> The root cause is that, selinux_inode_alloc_security has been deleted
> firstly from security_hook_heads, so security_inode_alloc directly
> return 0, that means the value of pointer inode->i_security equalling
> to NULL.
>
> But selinux_socket_post_create hasn't been deleted at that moment, so
> which would involked by mistake. Inside the function, pointer isec
> needs to point to inode->i_security, then a NULL pointer defect happens.
>
> For current upstream kernel, because of commit
> afb1cbe37440c7f38b9cf46fc331cc9dfd5cce21
> the inode security has been moved out to LSM infrastructure from
> individual security modules like selinux.
>
> But this patch still can be applied for solving similar issue when
> security_delete_hooks has been used. Also for stable branch v4.19,
> the inode security still need to be created in individual modules.
>
> The patch has been verified by Lenovo SR850P server through overnight
> reboot cycles.
>
> Signed-off-by: Huaisheng Ye <[email protected]>
> ---
> include/linux/lsm_hooks.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 20d8cf1..57cb2ac 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -2164,7 +2164,7 @@ static inline void security_delete_hooks(struct security_hook_list *hooks,
> int i;
>
> for (i = 0; i < count; i++)
> - hlist_del_rcu(&hooks[i].list);
> + hlist_del_rcu(&hooks[count - 1 - i].list);
> }
> #endif /* CONFIG_SECURITY_SELINUX_DISABLE */
>
>

--
James Morris
<[email protected]>

2020-01-09 03:24:03

by Huaisheng HS1 Ye

[permalink] [raw]
Subject: RE: [External] Re: [PATCH] LSM: Delete hooks in reverse order for avoiding race


> -----Original Message-----
> From: Ondrej Mosnacek <[email protected]>
> Sent: Wednesday, January 8, 2020 7:43 PM
>
> Hi,
>
> On Wed, Jan 8, 2020 at 9:51 AM Huaisheng Ye <[email protected]> wrote:
> > From: Huaisheng Ye <[email protected]>
> >
> > There is small possibility as race condition when selinux_disable has
> > been triggered. security_delete_hooks deletes all selinux hooks from
> > security_hook_heads, but there are some selinux functions which are
> > being called at the same time.
> >
> > Here is a panic accident scene from 4.18 based kernel,
> >
> > [ 26.654494] SELinux: Disabled at runtime.
> > [ 26.654507] BUG: unable to handle kernel NULL pointer dereference
> > at 0000000000000020
> > [ 26.654508] PGD 0 P4D 0
> > [ 26.654510] Oops: 0002 [#1] SMP NOPTI
> > [ 26.654512] CPU: 53 PID: 2614 Comm: systemd-cgroups Tainted: G
> > OE --------- - - 4.18.0-80.el8.x86_64 #1
> > [ 26.654512] Hardware name: Lenovo ThinkSystem SR850P
> > -[7D2H]-/-[7D2H]-, BIOS -[TEE145P-1.10]- 12/06/2019
> > [ 26.654519] RIP: 0010:selinux_socket_post_create+0x80/0x390
> > [ 26.654520] Code: e9 95 6a 89 00 bd 16 00 00 00 c7 44 24 04 01
> > 00 00 00 45 85 c0 0f 85 f6 00 00 00 8b 56 14 85 d2 0f 84 26 01 00
> > 00 89 54 24 04 <66> 41 89 6c 24 20 31 c0 41 89 54 24 1c 41 c6 44
> > 24 22 01 49 8b 4d
> > [ 26.654521] RSP: 0018:ffffbf515cc63e48 EFLAGS: 00010246
> > [ 26.654522] RAX: 0000000000000000 RBX: 0000000000000001 RCX:
> 0000000000000019
> > [ 26.654522] RDX: 0000000000000001 RSI: 0000000000000001 RDI:
> ffffffffab46f680
> > [ 26.654523] RBP: 0000000000000019 R08: 0000000000000000 R09:
> ffffbf515cc63e4c
> > [ 26.654523] R10: 0000000000000000 R11: 0000000000000000 R12:
> 0000000000000000
> > [ 26.654524] R13: ffff97d7bb6cbc80 R14: 0000000000000001 R15:
> ffff97d7bb6cbc80
> > [ 26.654525] FS: 00007f5c608ea380(0000) GS:ffff97d7bf140000(0000)
> knlGS:0000000000000000
> > [ 26.654525] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [ 26.654526] CR2: 0000000000000020 CR3: 0000011ebc934004 CR4:
> 00000000007606e0
> > [ 26.654527] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> 0000000000000000
> > [ 26.654528] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
> 0000000000000400
> > [ 26.654528] PKRU: 55555554
> > [ 26.654528] Call Trace:
> > [ 26.654535] security_socket_post_create+0x42/0x60
> > [ 26.654537] SELinux: Unregistering netfilter hooks
> > [ 26.654542] __sock_create+0x106/0x1a0
> > [ 26.654545] __sys_socket+0x57/0xe0
> > [ 26.654547] __x64_sys_socket+0x16/0x20
> > [ 26.654551] do_syscall_64+0x5b/0x1b0
> > [ 26.654554] entry_SYSCALL_64_after_hwframe+0x65/0xca
> >
> > The root cause is that, selinux_inode_alloc_security has been deleted
> > firstly from security_hook_heads, so security_inode_alloc directly
> > return 0, that means the value of pointer inode->i_security equalling
> > to NULL.
> >
> > But selinux_socket_post_create hasn't been deleted at that moment, so
> > which would involked by mistake. Inside the function, pointer isec
> > needs to point to inode->i_security, then a NULL pointer defect happens.
> >
> > For current upstream kernel, because of commit
> > afb1cbe37440c7f38b9cf46fc331cc9dfd5cce21
> > the inode security has been moved out to LSM infrastructure from
> > individual security modules like selinux.
> >
> > But this patch still can be applied for solving similar issue when
> > security_delete_hooks has been used. Also for stable branch v4.19, the
> > inode security still need to be created in individual modules.
>
> Thank you for the patch, however there are already existing proposed patches to
> fix this issue, see [1], [2], and [3]. At the moment it looks like the SELinux
> hooks reorder approach ([1]) will be accepted as a temporary solution (the SELinux
> runtime disable is being deprecated [4] in favor of properly disabling SELinux
> by setting
> selinux=0 on the kernel command line).

Hi Ondrej,

Thanks for introduction, and yes "selinux=0" is the simplest and effective workaround
for this defect.
I used it before as workaround and the panic issue couldn't be reproduced again.

>
> Your approach unfortunately isn't robust (depends on assumptions about how hooks
> are ordered by LSMs) nor complete (even the inverse order still has some race
> conditions that may lead to a crash - e.g.
> selinux_bpf_map() vs. selinux_bpf_map_alloc()).
>
> Also, please, don't forget to Cc the LSM/SELinux mailing lists
> ([email protected]/[email protected],
> respectively) for patches related to the LSM framework/SELinux.

Got it, I am new to selinux module.

Cheers,
Huaisheng Ye
Lenovo

>
> [1]
> https://lore.kernel.org/selinux/[email protected]/
> T/
> [2]
> https://lore.kernel.org/selinux/[email protected]/
> T/
> [3]
> https://lore.kernel.org/selinux/[email protected]/
> T/
> [4]
> https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git/commit/?
> h=next&id=89b223bfb8a89731bea4c84982b5d2ad7ba460e3
>
> >
> > The patch has been verified by Lenovo SR850P server through overnight
> > reboot cycles.
> >
> > Signed-off-by: Huaisheng Ye <[email protected]>
> > ---
> > include/linux/lsm_hooks.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > index 20d8cf1..57cb2ac 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -2164,7 +2164,7 @@ static inline void security_delete_hooks(struct
> security_hook_list *hooks,
> > int i;
> >
> > for (i = 0; i < count; i++)
> > - hlist_del_rcu(&hooks[i].list);
> > + hlist_del_rcu(&hooks[count - 1 - i].list);
> > }
> > #endif /* CONFIG_SECURITY_SELINUX_DISABLE */
> >
> > --
> > 1.8.3.1
> >
> >
>
> --
> Ondrej Mosnacek <omosnace at redhat dot com> Software Engineer, Security
> Technologies Red Hat, Inc.