lkml.org 
[lkml]   [2018]   [Jan]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH] general protection fault in sock_has_perm
    On Thu, Jan 18, 2018 at 4:58 PM, Mark Salyzyn <salyzyn@android.com> wrote:
    > general protection fault: 0000 [#1] PREEMPT SMP KASAN
    > CPU: 1 PID: 14233 Comm: syz-executor2 Not tainted 4.4.112-g5f6325b #28
    > task: ffff8801d1095f00 task.stack: ffff8800b5950000
    > RIP: 0010:[<ffffffff81b69b7e>] [<ffffffff81b69b7e>] sock_has_perm+0x1fe/0x3e0 security/selinux/hooks.c:4069
    > RSP: 0018:ffff8800b5957ce0 EFLAGS: 00010202
    > RAX: dffffc0000000000 RBX: 1ffff10016b2af9f RCX: ffffffff81b69b51
    > RDX: 0000000000000002 RSI: 0000000000000000 RDI: 0000000000000010
    > RBP: ffff8800b5957de0 R08: 0000000000000001 R09: 0000000000000001
    > R10: 0000000000000000 R11: 1ffff10016b2af68 R12: ffff8800b5957db8
    > R13: 0000000000000000 R14: ffff8800b7259f40 R15: 00000000000000d7
    > FS: 00007f72f5ae2700(0000) GS:ffff8801db300000(0000) knlGS:0000000000000000
    > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    > CR2: 0000000000a2fa38 CR3: 00000001d7980000 CR4: 0000000000160670
    > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
    > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
    > Stack:
    > ffffffff81b69a1f ffff8800b5957d58 00008000b5957d30 0000000041b58ab3
    > ffffffff83fc82f2 ffffffff81b69980 0000000000000246 ffff8801d1096770
    > ffff8801d3165668 ffffffff8157844b ffff8801d1095f00
    > ffff880000000001
    > Call Trace:
    > [<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()?

    > Signed-off-by: Mark Salyzyn <salyzyn@android.com>
    > Cc: Paul Moore <paul@paul-moore.com>
    > Cc: Stephen Smalley <sds@tycho.nsa.gov>
    > Cc: Eric Paris <eparis@parisplace.org>
    > Cc: James Morris <james.l.morris@oracle.com>
    > Cc: "Serge E. Hallyn" <serge@hallyn.com>
    > Cc: selinux@tycho.nsa.gov
    > Cc: linux-security-module@vger.kernel.org
    > Cc: linux-kernel@vger.kernel.org
    > Cc: stable@vger.kernel.org
    > ---
    > This patch should be applied to all stable trees (author wants
    > minimum of 3.18, 4.4, 4.9 and 4.14)
    >
    > security/selinux/hooks.c | 2 +-
    > 1 file changed, 1 insertion(+), 1 deletion(-)
    >
    > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
    > index 8644d864e3c1..95d7c8143373 100644
    > --- a/security/selinux/hooks.c
    > +++ b/security/selinux/hooks.c
    > @@ -4342,7 +4342,7 @@ static int sock_has_perm(struct sock *sk, u32 perms)
    > struct common_audit_data ad;
    > struct lsm_network_audit net = {0,};
    >
    > - if (sksec->sid == SECINITSID_KERNEL)
    > + if (!sksec || sksec->sid == SECINITSID_KERNEL)
    > return 0;
    >
    > ad.type = LSM_AUDIT_DATA_NET;
    > --
    > 2.16.0.rc1.238.g530d649a79-goog

    --
    paul moore
    www.paul-moore.com

    \
     
     \ /
      Last update: 2018-01-18 23:37    [W:3.762 / U:0.040 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site