From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ursula Braun Subject: Re: [PATCH net 1/4] net/smc: take sock lock in smc_ioctl() Date: Mon, 16 Jul 2018 13:56:42 +0200 Message-ID: <11a146f0-eebc-ece9-ed2b-32ad9a32a687@linux.ibm.com> References: <20180716100101.79272-1-ubraun@linux.ibm.com> <20180716120915.09d35dc0@epycfail> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: davem@davemloft.net, netdev@vger.kernel.org, linux-s390@vger.kernel.org, schwidefsky@de.ibm.com, heiko.carstens@de.ibm.com, raspl@linux.ibm.com, linux-kernel@vger.kernel.org, eric.dumazet@gmail.com, lifeasageek@gmail.com To: Stefano Brivio Return-path: Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:53790 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728872AbeGPMX4 (ORCPT ); Mon, 16 Jul 2018 08:23:56 -0400 Received: from pps.filterd (m0098414.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.22/8.16.0.22) with SMTP id w6GBshax025992 for ; Mon, 16 Jul 2018 07:56:49 -0400 Received: from e06smtp07.uk.ibm.com (e06smtp07.uk.ibm.com [195.75.94.103]) by mx0b-001b2d01.pphosted.com with ESMTP id 2k8rgf5vqq-1 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NOT) for ; Mon, 16 Jul 2018 07:56:49 -0400 Received: from localhost by e06smtp07.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 16 Jul 2018 12:56:47 +0100 In-Reply-To: <20180716120915.09d35dc0@epycfail> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 07/16/2018 12:09 PM, Stefano Brivio wrote: > On Mon, 16 Jul 2018 12:01:01 +0200 > Ursula Braun wrote: > >> From: Ursula Braun >> >> SMC ioctl processing requires the sock lock to work properly in >> all thinkable scenarios. >> Problem has been found with RaceFuzzer and fixes: >> KASAN: null-ptr-deref Read in smc_ioctl >> >> Reported-by: Byoungyoung Lee >> Reported-by: syzbot+35b2c5aa76fd398b9fd4@syzkaller.appspotmail.com >> Signed-off-by: Ursula Braun >> --- >> net/smc/af_smc.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c >> index 5334157f5065..a4381b38a521 100644 >> --- a/net/smc/af_smc.c >> +++ b/net/smc/af_smc.c >> @@ -1524,6 +1524,7 @@ static int smc_ioctl(struct socket *sock, unsigned int cmd, >> return -EBADF; >> return smc->clcsock->ops->ioctl(smc->clcsock, cmd, arg); >> } >> + lock_sock(&smc->sk); >> switch (cmd) { >> case SIOCINQ: /* same as FIONREAD */ >> if (smc->sk.sk_state == SMC_LISTEN) > > return -EINVAL; > > you should also unlock here, and: > > case SIOCOUTQ: > /* output queue size (not send + not acked) */ > if (smc->sk.sk_state == SMC_LISTEN) > return -EINVAL; > > here, and: > > case SIOCOUTQNSD: > /* output queue size (not send only) */ > if (smc->sk.sk_state == SMC_LISTEN) > return -EINVAL; > > here, and: > > case SIOCATMARK: > if (smc->sk.sk_state == SMC_LISTEN) > return -EINVAL; > > here. > sorry, my fault! V2 is on its way. Thanks for your hint.