On 01/18/2018 02:36 PM, Paul Moore wrote:
On Thu, Jan 18, 2018 at 4:58 PM, Mark Salyzyn <salyzyn@xxxxxxxxxxx> wrote:
general protection fault: 0000 [#1] PREEMPT SMP KASAN
CPU: 1 PID: 14233 Comm: syz-executor2 Not tainted 4.4.112-g5f6325b #28
. . .
[<ffffffff81b6a19d>] selinux_socket_setsockopt+0x4d/0x80 security/selinux/hooks.c:4338
[<ffffffff81b4873d>] security_socket_setsockopt+0x7d/0xb0 security/security.c:1257
[<ffffffff82df1ac8>] SYSC_setsockopt net/socket.c:1757 [inline]
[<ffffffff82df1ac8>] SyS_setsockopt+0xe8/0x250 net/socket.c:1746
[<ffffffff83776499>] entry_SYSCALL_64_fastpath+0x16/0x92
Code: c2 42 9b b6 81 be 01 00 00 00 48 c7 c7 a0 cb 2b 84 e8
f7 2f 6d ff 49 8d 7d 10 48 b8 00 00 00 00 00 fc ff df 48 89
fa 48 c1 ea 03 <0f> b6 04 02 84 c0 74 08 3c 03 0f 8e 83 01 00
00 41 8b 75 10 31
RIP [<ffffffff81b69b7e>] sock_has_perm+0x1fe/0x3e0 security/selinux/hooks.c:4069
RSP <ffff8800b5957ce0>
---[ end trace 7b5aaf788fef6174 ]---
In the absence of commit a4298e4522d6 ("net: add SOCK_RCU_FREE socket
flag") and all the associated infrastructure changes to take advantage
of a RCU grace period before freeing, there is a heightened
possibility that a security check is performed while an ill-timed
setsockopt call races in from user space. It then is prudent to null
check sk_security, and if the case, reject the permissions.
This adjustment is orthogonal to infrastructure improvements that may
nullify the needed check, but should be added as good code hygiene.
I'm skeptical that this is the full solution for systems that lack the
SOCK_RCU_FREE protection. Is this really limited to just
setsockopt()?
Maybe overstepped in my analysis and assumptions?
This is a result of a fuzzer hitting an android 4.4 KASAN kernel. We (so
far) have _not_ seen this with an android 4.9 KASAN kernel (which has
the SOCK_RCU_FREE adjustments). There is no standalone duplication or
PoC _except_ via the fuzzer. The rest of the statements stands based on
this tidbit (statements of general good code hygiene, not 100% sure
SOCK_RCU_FREE usage is completely covered, KISS solution etc).
To be honest, yes, this may be a layer in the onion (swat this NULL
check does not by itself solve the _problem_), I'd prefer kernel
continuing on in a rational manner rather than panic ... and I have a
gut feeling this could be a gratuitous NULL check if all the bugs in the
network layer have been solved <that may be sarcasm, I can not tell>.
Programming to solve a problem with one's gut is not a good practice,
but hygiene is. This is 10 characters, and an estimated 1.2ns of added
hygiene.
No, I do not think this is limited to setsockopt, but would be willing
to believe a multithreaded attack of any socket functions or ioctl would
drop down to the check with sock_has_perm at possibly the wrong time in
socket teardown.
Sincerely -- Mark Salyzyn