From mboxrd@z Thu Jan 1 00:00:00 1970 Date: Mon, 3 Apr 2017 11:10:42 +1000 (AEST) From: James Morris To: Paul Moore cc: Stephen Smalley , Dan Carpenter , Markus Elfring , Eric Paris , James Morris , "Serge E. Hallyn" , William Roberts , selinux@tycho.nsa.gov, linux-security-module@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: Re: [PATCH] selinux: Fix an uninitialized variable bug In-Reply-To: Message-ID: References: <20170331152118.GA8141@mwanda> <1490975541.31110.12.camel@tycho.nsa.gov> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII List-Id: "Security-Enhanced Linux \(SELinux\) mailing list" List-Post: List-Help: On Fri, 31 Mar 2017, Paul Moore wrote: > On Fri, Mar 31, 2017 at 11:52 AM, Stephen Smalley wrote: > > On Fri, 2017-03-31 at 18:21 +0300, Dan Carpenter wrote: > >> We removed this initialization as a cleanup but it is probably > >> required. > >> > >> The concern is that "nel" can be zero. I'm not an expert on SELinux > >> code but I think it looks possible to write an SELinux policy which > >> triggers this bug. GCC doesn't catch this, but my static checker > >> does. > >> > >> Fixes: 9c312e79d6af ("selinux: Delete an unnecessary variable > >> initialisation in range_read()") > >> Signed-off-by: Dan Carpenter > > > > Nice catch, thanks! > > > > Acked-by: Stephen Smalley > > Yes, indeed. Thanks Dan, I should have caught this when merging Markus' patch. > I'd like to reiterate that I generally don't want to accept cleanup patches into the security tree from Markus (or indeed from others who only do cleanup/whitespace work). See https://lkml.org/lkml/2017/1/29/172, and please click through and read Dan's comments. All patches carry risks of introducing new bugs, and kernel "cleanup: patches generally offer a pretty high cost/benefit ratio. If such patches come from core developers of that code, or from kernel developers with experience in *analyzing and fixing* bugs, that's very different. Paul, please review all of these patches very carefully before sending your pull request. -- James Morris From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Morris Date: Mon, 03 Apr 2017 01:10:42 +0000 Subject: Re: [PATCH] selinux: Fix an uninitialized variable bug Message-Id: List-Id: References: <20170331152118.GA8141@mwanda> <1490975541.31110.12.camel@tycho.nsa.gov> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: linux-security-module@vger.kernel.org On Fri, 31 Mar 2017, Paul Moore wrote: > On Fri, Mar 31, 2017 at 11:52 AM, Stephen Smalley wrote: > > On Fri, 2017-03-31 at 18:21 +0300, Dan Carpenter wrote: > >> We removed this initialization as a cleanup but it is probably > >> required. > >> > >> The concern is that "nel" can be zero. I'm not an expert on SELinux > >> code but I think it looks possible to write an SELinux policy which > >> triggers this bug. GCC doesn't catch this, but my static checker > >> does. > >> > >> Fixes: 9c312e79d6af ("selinux: Delete an unnecessary variable > >> initialisation in range_read()") > >> Signed-off-by: Dan Carpenter > > > > Nice catch, thanks! > > > > Acked-by: Stephen Smalley > > Yes, indeed. Thanks Dan, I should have caught this when merging Markus' patch. > I'd like to reiterate that I generally don't want to accept cleanup patches into the security tree from Markus (or indeed from others who only do cleanup/whitespace work). See https://lkml.org/lkml/2017/1/29/172, and please click through and read Dan's comments. All patches carry risks of introducing new bugs, and kernel "cleanup: patches generally offer a pretty high cost/benefit ratio. If such patches come from core developers of that code, or from kernel developers with experience in *analyzing and fixing* bugs, that's very different. Paul, please review all of these patches very carefully before sending your pull request. -- James Morris From mboxrd@z Thu Jan 1 00:00:00 1970 From: jmorris@namei.org (James Morris) Date: Mon, 3 Apr 2017 11:10:42 +1000 (AEST) Subject: [PATCH] selinux: Fix an uninitialized variable bug In-Reply-To: References: <20170331152118.GA8141@mwanda> <1490975541.31110.12.camel@tycho.nsa.gov> Message-ID: To: linux-security-module@vger.kernel.org List-Id: linux-security-module.vger.kernel.org On Fri, 31 Mar 2017, Paul Moore wrote: > On Fri, Mar 31, 2017 at 11:52 AM, Stephen Smalley wrote: > > On Fri, 2017-03-31 at 18:21 +0300, Dan Carpenter wrote: > >> We removed this initialization as a cleanup but it is probably > >> required. > >> > >> The concern is that "nel" can be zero. I'm not an expert on SELinux > >> code but I think it looks possible to write an SELinux policy which > >> triggers this bug. GCC doesn't catch this, but my static checker > >> does. > >> > >> Fixes: 9c312e79d6af ("selinux: Delete an unnecessary variable > >> initialisation in range_read()") > >> Signed-off-by: Dan Carpenter > > > > Nice catch, thanks! > > > > Acked-by: Stephen Smalley > > Yes, indeed. Thanks Dan, I should have caught this when merging Markus' patch. > I'd like to reiterate that I generally don't want to accept cleanup patches into the security tree from Markus (or indeed from others who only do cleanup/whitespace work). See https://lkml.org/lkml/2017/1/29/172, and please click through and read Dan's comments. All patches carry risks of introducing new bugs, and kernel "cleanup: patches generally offer a pretty high cost/benefit ratio. If such patches come from core developers of that code, or from kernel developers with experience in *analyzing and fixing* bugs, that's very different. Paul, please review all of these patches very carefully before sending your pull request. -- James Morris -- 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