From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751991AbdBINML (ORCPT ); Thu, 9 Feb 2017 08:12:11 -0500 Received: from mx02-sz.bfs.de ([194.94.69.103]:58569 "EHLO mx02-sz.bfs.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751040AbdBINMJ (ORCPT ); Thu, 9 Feb 2017 08:12:09 -0500 Message-ID: <589C69F7.8030603@bfs.de> Date: Thu, 09 Feb 2017 14:09:11 +0100 From: walter harms Reply-To: wharms@bfs.de User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; de; rv:1.9.1.16) Gecko/20101125 SUSE/3.0.11 Thunderbird/3.0.11 MIME-Version: 1.0 To: Colin King CC: Casey Schaufler , James Morris , "Serge E . Hallyn" , linux-security-module@vger.kernel.org, kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] Smack: fix a dereference before null check on sock->sk References: <20170209120301.28152-1-colin.king@canonical.com> In-Reply-To: <20170209120301.28152-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 Am 09.02.2017 13:03, schrieb Colin King: > 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. > > Detected with CoverityScan, CID#1324196 ("Dereference before null check") > > Signed-off-by: Colin Ian King > --- > security/smack/smack_lsm.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c > index fc8fb31..bb17387 100644 > --- a/security/smack/smack_lsm.c > +++ b/security/smack/smack_lsm.c > @@ -2899,7 +2899,7 @@ static int smack_socket_connect(struct socket *sock, struct sockaddr *sap, > #endif > #ifdef SMACK_IPV6_SECMARK_LABELING > struct smack_known *rsp; > - struct socket_smack *ssp = sock->sk->sk_security; > + struct socket_smack *ssp; > #endif > > if (sock->sk == NULL) > @@ -2916,9 +2916,11 @@ static int smack_socket_connect(struct socket *sock, struct sockaddr *sap, > return -EINVAL; > #ifdef SMACK_IPV6_SECMARK_LABELING > rsp = smack_ipv6host_label(sip); > - if (rsp != NULL) > + if (rsp != NULL) { > + 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); In the hope of reducing the ifdef forrest: you could move the struct smack_known *rsp; and struct socket_smack *ssp; into the block like: { struct smack_known *rsp=smack_ipv6host_label(sip); if (rsp != NULL) { struct socket_smack *ssp= sock->sk->sk_security; rc = smk_ipv6_check(ssp->smk_out, rsp, sip, SMK_CONNECTING); } } just my 2 cents ... re, wh