From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753866AbdBISsG (ORCPT ); Thu, 9 Feb 2017 13:48:06 -0500 Received: from nm7-vm4.bullet.mail.ne1.yahoo.com ([98.138.91.167]:55779 "EHLO nm7-vm4.bullet.mail.ne1.yahoo.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753536AbdBISry (ORCPT ); Thu, 9 Feb 2017 13:47:54 -0500 X-Yahoo-Newman-Id: 532917.2788.bm@smtp204.mail.ne1.yahoo.com X-Yahoo-Newman-Property: ymail-3 X-YMail-OSG: 5QHDfKAVM1m7t5KOqFTuO7Pt8sSG_KIWu.N5PkjKZb.dVZX cCNbK_pLw1wI9hAIAH0s.auIjcVuyb_wFulszkboyIJmhLfcu4q.ac.CaH2z FBS7jLwPd1g_T2If4Sbwuwiny0aUtLkzS03iw7h77K4zyPnir0mOawV7DoBP UlnFNLY1S9p.X5ycRsXjVSaTOxS6giFIjOrmmQrEQgP21bnu0qiUvOi9krCq HlNtHAKvVd.mDjrkeVbMFHxrLlCp_uX6nzwSpgFVNjixh5o18IxbhxkaaeqU G3hAiBA8sbAG6F_fdUH0zHfWyra9pWQFH3gtK25CyaqN3_aT5AsdxtDB7RAE uMzDQhFl8umKr0gbp51Q5idBb3Mj4rNnNJF2KYiattHZqP0.FbMDg5cqfARX XWQzsSixWwUIZx4_CM_8xtncezj3EK7VGwA5EhbjY.8N_Qw0.ehJqz8Tibkx ITGwGl.UPLrPWiD8zlHhaZBRprmcFDpsS3ZcpSsiErJIHIFNYPf9sjZ__B4x xw365GQ2xi3woOvx7G7GEe9EWBav6MA4SlUUDwcj8eJ4CXII0UjE- X-Yahoo-SMTP: OIJXglSswBDfgLtXluJ6wiAYv6_cnw-- Subject: Re: [PATCH][V2] Smack: fix a dereference before null check on sock->sk To: Colin King , James Morris , "Serge E . Hally" , linux-security-module@vger.kernel.org References: <20170209171127.5029-1-colin.king@canonical.com> Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org From: Casey Schaufler Message-ID: Date: Thu, 9 Feb 2017 10:44:53 -0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <20170209171127.5029-1-colin.king@canonical.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2/9/2017 9:11 AM, Colin King wrote: > From: Colin Ian King > > The initialisation of pointer ssp is from a dereference on sock->sk > before sock-sk is null checked, hence there is a potential for a > null pointer deference. Fix this by moving the assignment of ssp > to just before it is used in the call to smk_ipv6_check. > > Also minor clean up of code to reduce #ifdef noise. > Detected with CoverityScan, CID#1324196 ("Dereference before null check") > > Signed-off-by: Colin Ian King > --- > security/smack/smack_lsm.c | 19 +++++++++++-------- > 1 file changed, 11 insertions(+), 8 deletions(-) > > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c > index fc8fb31..0c5656d 100644 > --- a/security/smack/smack_lsm.c > +++ b/security/smack/smack_lsm.c > @@ -2897,10 +2897,6 @@ static int smack_socket_connect(struct socket *sock, struct sockaddr *sap, > #if IS_ENABLED(CONFIG_IPV6) > struct sockaddr_in6 *sip = (struct sockaddr_in6 *)sap; > #endif > -#ifdef SMACK_IPV6_SECMARK_LABELING > - struct smack_known *rsp; > - struct socket_smack *ssp = sock->sk->sk_security; > -#endif > > if (sock->sk == NULL) > return 0; > @@ -2915,10 +2911,17 @@ static int smack_socket_connect(struct socket *sock, struct sockaddr *sap, > if (addrlen < sizeof(struct sockaddr_in6)) > return -EINVAL; > #ifdef SMACK_IPV6_SECMARK_LABELING > - rsp = smack_ipv6host_label(sip); > - if (rsp != NULL) > - rc = smk_ipv6_check(ssp->smk_out, rsp, sip, > - SMK_CONNECTING); > + { > + struct smack_known *rsp = smack_ipv6host_label(sip); I make a habit of not using nested declarations. I appreciate the intent of the patch, but I would rather take a more global approach to the cleanup. > + > + if (rsp != NULL) { > + struct socket_smack *ssp = > + sock->sk->sk_security; > + > + rc = smk_ipv6_check(ssp->smk_out, rsp, sip, > + SMK_CONNECTING); > + } > + } > #endif > #ifdef SMACK_IPV6_PORT_LABELING > rc = smk_ipv6_port_check(sock->sk, sip, SMK_CONNECTING);