From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Cyrus-Session-Id: sloti22d1t05-142065-1516384021-2-12245528138378318899 X-Sieve: CMU Sieve 3.0 X-Spam-known-sender: no X-Spam-score: 0.0 X-Spam-hits: BAYES_00 -1.9, HEADER_FROM_DIFFERENT_DOMAINS 0.25, RCVD_IN_DNSWL_HI -5, T_RP_MATCHES_RCVD -0.01, LANGUAGES en, BAYES_USED global, SA_VERSION 3.4.0 X-Spam-source: IP='209.132.180.67', Host='vger.kernel.org', Country='US', FromHeader='com', MailFrom='org' X-Spam-charsets: plain='utf-8' X-Resolved-to: greg@kroah.com X-Delivered-to: greg@kroah.com X-Mail-from: stable-owner@vger.kernel.org ARC-Seal: i=1; a=rsa-sha256; cv=none; d=messagingengine.com; s=arctest; t=1516384020; b=DsjK/98tGDmZmMLtVcRrRwJq1/N02dazUU6vwoh1eaT3gUS cIItZWscF5Uw58EN6fouwE8ObFWzvF82cUcQmkrDkBeHCsR/97p4hFRlBhorMxw6 g4KuAMRmhnAY/NtW/NY+NK2rWqkitSL8ihVHERMW2bEWEEd69gwM/5DvF9iOmMxa y6YjYgxg0jOnn9LzwFwqAoU/o8vd/lpTg/nqIeJNh/ZvRAlHk8AK08Lp2/a+ZWeV 8xh0r/g9GuZzPgAe8gBdT2ZSpwJ8T4aSwVEcCrm6fxc0jA9emGZBYPFe9KJPcpg9 3ShyD3F+HcerWnOEamP8LY71CMcWDlI/9GFS2xg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=subject:to:cc:references:from:message-id :date:mime-version:in-reply-to:content-type :content-transfer-encoding:sender:list-id; s=arctest; t= 1516384020; bh=bSyZPRrjtu0CuYKiy/LmxewPICs0fAcpC+IMQEauLM4=; b=b oEN2awAG52IMBoIuGKXYbgkgHaVfghBD4hlO7ZPqg222KKHFEFNbUYDlEBpce8cF RiXjtXT7ZCx/ZsK8Fqk5gNrhmCXCOVJyEPNADFY+PSeL6QhkQHAFEi8cdKKnl+D0 HfZ/T6bcMnNhEhbh19ax17277w6HwdYy2EkwosQEqXo93ltWdcfk5uUekb6BFuZ+ 03xV93LqevA8YJFc+T78b33yh8BCN3VZGrOWixmTbEIa9twdWS4DajWJI0HX2EqC QM0dncD6KfzxsQZsyH0AHjcS18CDByNV5rdd0a8EtRVTveSf9ancMUuas6DDqkzj UbKhnOQkgM3DmYkmh1OtQ== ARC-Authentication-Results: i=1; mx3.messagingengine.com; arc=none (no signatures found); dkim=pass (2048-bit rsa key sha256) header.d=android.com header.i=@android.com header.b=KHZmBRCo x-bits=2048 x-keytype=rsa x-algorithm=sha256 x-selector=20161025; dmarc=pass (p=none,has-list-id=yes,d=none) header.from=android.com; iprev=pass policy.iprev=209.132.180.67 (vger.kernel.org); spf=none smtp.mailfrom=stable-owner@vger.kernel.org smtp.helo=vger.kernel.org; x-aligned-from=fail; x-google-dkim=pass (2048-bit rsa key) header.d=1e100.net header.i=@1e100.net header.b=BpxGcmCT; x-ptr=pass x-ptr-helo=vger.kernel.org x-ptr-lookup=vger.kernel.org; x-return-mx=pass smtp.domain=vger.kernel.org smtp.result=pass smtp_org.domain=kernel.org smtp_org.result=pass smtp_is_org_domain=no header.domain=android.com header.result=pass header_is_org_domain=yes Authentication-Results: mx3.messagingengine.com; arc=none (no signatures found); dkim=pass (2048-bit rsa key sha256) header.d=android.com header.i=@android.com header.b=KHZmBRCo x-bits=2048 x-keytype=rsa x-algorithm=sha256 x-selector=20161025; dmarc=pass (p=none,has-list-id=yes,d=none) header.from=android.com; iprev=pass policy.iprev=209.132.180.67 (vger.kernel.org); spf=none smtp.mailfrom=stable-owner@vger.kernel.org smtp.helo=vger.kernel.org; x-aligned-from=fail; x-google-dkim=pass (2048-bit rsa key) header.d=1e100.net header.i=@1e100.net header.b=BpxGcmCT; x-ptr=pass x-ptr-helo=vger.kernel.org x-ptr-lookup=vger.kernel.org; x-return-mx=pass smtp.domain=vger.kernel.org smtp.result=pass smtp_org.domain=kernel.org smtp_org.result=pass smtp_is_org_domain=no header.domain=android.com header.result=pass header_is_org_domain=yes Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932536AbeASRq7 (ORCPT ); Fri, 19 Jan 2018 12:46:59 -0500 Received: from mail-pf0-f175.google.com ([209.85.192.175]:42274 "EHLO mail-pf0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932523AbeASRq5 (ORCPT ); Fri, 19 Jan 2018 12:46:57 -0500 X-Google-Smtp-Source: ACJfBosS/6W+63jfck3rb7Y/zWlfs4SUS1ynO+GftIVfM1W0BHrhnCVdYYeLDBWtWiOl8YYY+JWPmQ== Subject: Re: [PATCH] general protection fault in sock_has_perm To: Paul Moore Cc: linux-kernel@vger.kernel.org, Stephen Smalley , Eric Paris , James Morris , "Serge E. Hallyn" , selinux@tycho.nsa.gov, linux-security-module@vger.kernel.org, stable@vger.kernel.org, Eric Dumazer References: <20180118215853.228182-1-salyzyn@android.com> <1f185712-9136-be88-02c0-5613a7683619@android.com> From: Mark Salyzyn Message-ID: <0cab9b55-b21e-48b6-82f3-3750f470ea5b@android.com> Date: Fri, 19 Jan 2018 09:46:55 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Sender: stable-owner@vger.kernel.org X-Mailing-List: stable@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-Mailing-List: linux-kernel@vger.kernel.org List-ID: On 01/19/2018 09:06 AM, Paul Moore wrote: > On Fri, Jan 19, 2018 at 10:49 AM, Mark Salyzyn wrote: >> On 01/18/2018 02:36 PM, Paul Moore wrote: >>> On Thu, Jan 18, 2018 at 4:58 PM, Mark Salyzyn wrote: >>>> general protection fault: 0000 [#1] PREEMPT SMP KASAN >>>> CPU: 1 PID: 14233 Comm: syz-executor2 Not tainted 4.4.112-g5f6325b #28 >>>> . . . >>>> [] selinux_socket_setsockopt+0x4d/0x80 >>>> security/selinux/hooks.c:4338 >>>> [] security_socket_setsockopt+0x7d/0xb0 >>>> security/security.c:1257 >>>> [] SYSC_setsockopt net/socket.c:1757 [inline] >>>> [] SyS_setsockopt+0xe8/0x250 net/socket.c:1746 >>>> [] 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 [] sock_has_perm+0x1fe/0x3e0 >>>> security/selinux/hooks.c:4069 >>>> RSP >>>> ---[ 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 . 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. > I'm not necessarily opposed to adding additional safety checks, if > warranted, but I am opposed to adding a single check and declaring > mission accomplished when there is a suspicion that additional checks > may be needed. > > Perhaps in this particular case it really is only setsockopt(), but > from what I can tell from your comments and the SOCK_RCU_FREE commit > message it would appear that there is a race condition here between a > socket's lifetime and its visibility to userspace. I will need to fix my comments to be clearer ... (besides, I got the error return wrong, so I will have to respin it anyways). In later kernels SOCK_RCU_FREE _appears_ to fix the race condition. In earlier kernels there is _no_ SOCK_RCU_FREE infrastructure, and thus the race condition exists. That race conditions _should_ have been solved in ToT. I have evaluated that porting all the SOCK_RCU_FREE refactoring may be too risky an endeavor for stable kernels though (maybe I am wrong about that? The author of those fixes edumazet@google.com has been notified, have yet to receive a response). Because I can not confirm that SOCK_RCU_FREE solves the problem, I am making a case for this to land in 4.9+ to ToT kernels as good hygiene. But maybe this should only go in 4.4- stable ... -- Mark From mboxrd@z Thu Jan 1 00:00:00 1970 From: salyzyn@android.com (Mark Salyzyn) Date: Fri, 19 Jan 2018 09:46:55 -0800 Subject: [PATCH] general protection fault in sock_has_perm In-Reply-To: References: <20180118215853.228182-1-salyzyn@android.com> <1f185712-9136-be88-02c0-5613a7683619@android.com> Message-ID: <0cab9b55-b21e-48b6-82f3-3750f470ea5b@android.com> To: linux-security-module@vger.kernel.org List-Id: linux-security-module.vger.kernel.org On 01/19/2018 09:06 AM, Paul Moore wrote: > On Fri, Jan 19, 2018 at 10:49 AM, Mark Salyzyn wrote: >> On 01/18/2018 02:36 PM, Paul Moore wrote: >>> On Thu, Jan 18, 2018 at 4:58 PM, Mark Salyzyn wrote: >>>> general protection fault: 0000 [#1] PREEMPT SMP KASAN >>>> CPU: 1 PID: 14233 Comm: syz-executor2 Not tainted 4.4.112-g5f6325b #28 >>>> . . . >>>> [] selinux_socket_setsockopt+0x4d/0x80 >>>> security/selinux/hooks.c:4338 >>>> [] security_socket_setsockopt+0x7d/0xb0 >>>> security/security.c:1257 >>>> [] SYSC_setsockopt net/socket.c:1757 [inline] >>>> [] SyS_setsockopt+0xe8/0x250 net/socket.c:1746 >>>> [] 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 [] sock_has_perm+0x1fe/0x3e0 >>>> security/selinux/hooks.c:4069 >>>> RSP >>>> ---[ 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 . 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. > I'm not necessarily opposed to adding additional safety checks, if > warranted, but I am opposed to adding a single check and declaring > mission accomplished when there is a suspicion that additional checks > may be needed. > > Perhaps in this particular case it really is only setsockopt(), but > from what I can tell from your comments and the SOCK_RCU_FREE commit > message it would appear that there is a race condition here between a > socket's lifetime and its visibility to userspace. I will need to fix my comments to be clearer ... (besides, I got the error return wrong, so I will have to respin it anyways). In later kernels SOCK_RCU_FREE _appears_ to fix the race condition. In earlier kernels there is _no_ SOCK_RCU_FREE infrastructure, and thus the race condition exists. That race conditions _should_ have been solved in ToT. I have evaluated that porting all the SOCK_RCU_FREE refactoring may be too risky an endeavor for stable kernels though (maybe I am wrong about that? The author of those fixes edumazet at google.com has been notified, have yet to receive a response). Because I can not confirm that SOCK_RCU_FREE solves the problem, I am making a case for this to land in 4.9+ to ToT kernels as good hygiene. But maybe this should only go in 4.4- stable ... -- Mark -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html