From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932194AbcGOHMI (ORCPT ); Fri, 15 Jul 2016 03:12:08 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:30536 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752582AbcGOHMF (ORCPT ); Fri, 15 Jul 2016 03:12:05 -0400 X-IBM-Helo: d06dlp02.portsmouth.uk.ibm.com X-IBM-MailFrom: ubraun@linux.vnet.ibm.com X-IBM-RcptTo: kernel-janitors@vger.kernel.org;linux-kernel@vger.kernel.org;linux-s390@vger.kernel.org Subject: Re: [patch] netiucv: silence an underflow warning To: Dan Carpenter References: <20160714113016.GJ18175@mwanda> Cc: Martin Schwidefsky , Heiko Carstens , linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org From: Ursula Braun Date: Fri, 15 Jul 2016 09:11:58 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <20160714113016.GJ18175@mwanda> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16071507-0016-0000-0000-0000020AC486 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16071507-0017-0000-0000-00002241CB9E Message-Id: <57888CBE.9050002@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-07-15_03:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1604210000 definitions=main-1607150082 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Dan, thanks for reporting this netiucv-problem. There is an implication: Without fix, buffer values between 2**31 and 2**32 are not detected as invalid values. Your fix would help, but since we have to touch the code, I suggest to modernize it, moving from simple_strtoul() to kstrtouint(). This would be the patch I have in mind: --- a/drivers/s390/net/netiucv.c +++ b/drivers/s390/net/netiucv.c @@ -1564,21 +1564,21 @@ static ssize_t buffer_write (struct devi { struct netiucv_priv *priv = dev_get_drvdata(dev); struct net_device *ndev = priv->conn->netdev; - char *e; - int bs1; + unsigned int bs1; + int rc; IUCV_DBF_TEXT(trace, 3, __func__); if (count >= 39) return -EINVAL; - bs1 = simple_strtoul(buf, &e, 0); + rc = kstrtouint(buf, 0, &bs1); - if (e && (!isspace(*e))) { - IUCV_DBF_TEXT_(setup, 2, "buffer_write: invalid char %02x\n", - *e); + if (rc == -EINVAL) { + IUCV_DBF_TEXT_(setup, 2, "buffer_write: invalid char %s\n", + buf); return -EINVAL; } - if (bs1 > NETIUCV_BUFSIZE_MAX) { + if ((rc == -ERANGE) || (bs1 > NETIUCV_BUFSIZE_MAX)) { IUCV_DBF_TEXT_(setup, 2, "buffer_write: buffer size %d too large\n", bs1); And a question about your static checker detecting this problem: Is it a private checker, or something we could use as well? Does your checker like my patch version? Thanks, Ursula On 07/14/2016 01:30 PM, Dan Carpenter wrote: > I haven't looked at the implications but we accidentally allow bs1 to > be negative. It makes my static checker complain. > > Signed-off-by: Dan Carpenter > > diff --git a/drivers/s390/net/netiucv.c b/drivers/s390/net/netiucv.c > index b0e8ffd..85a5744 100644 > --- a/drivers/s390/net/netiucv.c > +++ b/drivers/s390/net/netiucv.c > @@ -1578,7 +1578,7 @@ static ssize_t buffer_write (struct device *dev, struct device_attribute *attr, > *e); > return -EINVAL; > } > - if (bs1 > NETIUCV_BUFSIZE_MAX) { > + if (bs1 < 0 || bs1 > NETIUCV_BUFSIZE_MAX) { > IUCV_DBF_TEXT_(setup, 2, > "buffer_write: buffer size %d too large\n", > bs1); >