From patchwork Thu Mar 23 11:45:50 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dan Carpenter X-Patchwork-Id: 9640903 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 8E707602D6 for ; Thu, 23 Mar 2017 11:46:22 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7D7E927B81 for ; Thu, 23 Mar 2017 11:46:22 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 7234F284A5; Thu, 23 Mar 2017 11:46:22 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2398B27B81 for ; Thu, 23 Mar 2017 11:46:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933321AbdCWLqH (ORCPT ); Thu, 23 Mar 2017 07:46:07 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:25857 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932855AbdCWLqF (ORCPT ); Thu, 23 Mar 2017 07:46:05 -0400 Received: from aserv0022.oracle.com (aserv0022.oracle.com [141.146.126.234]) by userp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id v2NBjvbq025158 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 23 Mar 2017 11:45:57 GMT Received: from userv0122.oracle.com (userv0122.oracle.com [156.151.31.75]) by aserv0022.oracle.com (8.14.4/8.14.4) with ESMTP id v2NBjutD015041 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 23 Mar 2017 11:45:56 GMT Received: from abhmp0003.oracle.com (abhmp0003.oracle.com [141.146.116.9]) by userv0122.oracle.com (8.14.4/8.14.4) with ESMTP id v2NBjtU1019087; Thu, 23 Mar 2017 11:45:55 GMT Received: from mwanda (/154.0.138.2) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Thu, 23 Mar 2017 04:45:54 -0700 Date: Thu, 23 Mar 2017 14:45:50 +0300 From: Dan Carpenter To: Colin Ian King Cc: Johannes Thumshirn , "James E . J . Bottomley" , "Martin K . Petersen" , fcoe-devel@open-fcoe.org, linux-scsi@vger.kernel.org, kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] scsi: fcoe: sanity check string size for store_ctrl_mode option Message-ID: <20170323114549.GN32449@mwanda> References: <20170322140137.28485-1-colin.king@canonical.com> <20170322193945.GK32449@mwanda> <1b8384a7-9efe-7b93-e418-0f377bb84a73@canonical.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1b8384a7-9efe-7b93-e418-0f377bb84a73@canonical.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Source-IP: aserv0022.oracle.com [141.146.126.234] Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Mar 22, 2017 at 07:42:08PM +0000, Colin Ian King wrote: > On 22/03/17 19:39, Dan Carpenter wrote: > > On Wed, Mar 22, 2017 at 02:01:37PM +0000, Colin King wrote: > >> From: Colin Ian King > >> > >> Reading and writing to mode[count - 1] implies the count should not > >> be less than 1 so add a sanity check for this. > >> > >> Detected with CoverityScan, CID#1357345 ("Overflowed array index write") > >> > >> Signed-off-by: Colin Ian King > > > > This is harmless, of course, but count can't be zero. This is a sysfs > > file so we test for zero size writes in sysfs_kf_write() and return > > early. > > Ah, thanks for pointing out that. I overlooked that detail. > The only reason I know this stuff is because it's annotated in Smatch. So I do this: Then when I run kchecker drivers/scsi/fcoe/fcoe_sysfs.c, it tells me: drivers/scsi/fcoe/fcoe_sysfs.c:292 store_ctlr_mode() implied: count = '1-1000000000,2147479552' Which is sort of surprising... The 1000000000 value is a hack I made so that it would never complain that "off + count" will wrap. But apparently something has changed so it's also picking up the true limit of count which is 2147479552. Then I run the following commands to view the call tree: smdb.py store_ctlr_mode smdb.py dev_attr_store smdb.py sysfs_kf_write I have some vim macros so I can look these up really quickly. regards, dan carpenter diff --git a/drivers/scsi/fcoe/fcoe_sysfs.c b/drivers/scsi/fcoe/fcoe_sysfs.c index 9cf3d56296ab..c491ad8fb0a8 100644 --- a/drivers/scsi/fcoe/fcoe_sysfs.c +++ b/drivers/scsi/fcoe/fcoe_sysfs.c @@ -281,6 +281,7 @@ static ssize_t show_ctlr_mode(struct device *dev, "%s\n", name); } +#include "/home/dcarpenter/progs/smatch/devel/check_debug.h" static ssize_t store_ctlr_mode(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) @@ -288,6 +289,7 @@ static ssize_t store_ctlr_mode(struct device *dev, struct fcoe_ctlr_device *ctlr = dev_to_ctlr(dev); char mode[FCOE_MAX_MODENAME_LEN + 1]; + __smatch_implied(count); if (count > FCOE_MAX_MODENAME_LEN) return -EINVAL;