From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753013AbcLKAgy (ORCPT ); Sat, 10 Dec 2016 19:36:54 -0500 Received: from smtprelay0003.hostedemail.com ([216.40.44.3]:58433 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752691AbcLKAgw (ORCPT ); Sat, 10 Dec 2016 19:36:52 -0500 X-Session-Marker: 6A6F6540706572636865732E636F6D X-Spam-Summary: 50,0,0,,d41d8cd98f00b204,joe@perches.com,:::::::::::::::::,RULES_HIT:41:355:379:421:541:599:800:960:967:973:982:988:989:1260:1263:1277:1311:1313:1314:1345:1359:1373:1437:1515:1516:1518:1534:1543:1593:1594:1605:1711:1730:1747:1777:1792:2393:2525:2553:2561:2564:2682:2685:2693:2828:2859:2933:2937:2939:2942:2945:2947:2951:2954:3022:3138:3139:3140:3141:3142:3622:3865:3866:3867:3868:3870:3871:3872:3874:3934:3936:3938:3941:3944:3947:3950:3953:3956:3959:4184:4250:4321:5007:6117:6119:7903:8599:8660:9010:9025:10004:10400:10848:11026:11232:11658:11914:12043:12679:12740:12760:13095:13148:13230:13439:14096:14097:14180:14181:14659:14721:14849:21060:21080:21433:21451:30003:30012:30036:30054:30064:30070:30075:30090:30091,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:0:0,LFtime:2,LUA_SUMMARY:none X-HE-Tag: drop03_58c0fb5278e5e X-Filterd-Recvd-Size: 4529 Message-ID: <1481416608.1764.7.camel@perches.com> Subject: Re: [patch] nvme-fabrics: correct some printk information From: Joe Perches To: Julia Lawall , Dan Carpenter , Rasmus Villemoes Cc: James Smart , Keith Busch , Jens Axboe , linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Date: Sat, 10 Dec 2016 16:36:48 -0800 In-Reply-To: References: <20161210090618.GA11680@elgon.mountain> <1481369270.5946.51.camel@perches.com> <20161210184029.GY8176@mwanda> Content-Type: text/plain; charset="ISO-8859-1" X-Mailer: Evolution 3.22.1-0ubuntu2 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, 2016-12-10 at 21:06 +0100, Julia Lawall wrote: > > On Sat, 10 Dec 2016, Dan Carpenter wrote: > > > On Sat, Dec 10, 2016 at 03:27:50AM -0800, Joe Perches wrote: > > > On Sat, 2016-12-10 at 12:06 +0300, Dan Carpenter wrote: > > > > We really don't care where "ctrl" is on the stack since we're just > > > > returning soon what we want is the actual ctrl pointer itself. > > > > > > > > Signed-off-by: Dan Carpenter > > > > > > > > diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c > > > > > > [] > > > > @@ -2402,7 +2402,7 @@ enum blk_eh_timer_return > > > > > > > > dev_info(ctrl->ctrl.device, > > > > "NVME-FC{%d}: new ctrl: NQN \"%s\" (%p)\n", > > > > - ctrl->cnum, ctrl->ctrl.opts->subsysnqn, &ctrl); > > > > + ctrl->cnum, ctrl->ctrl.opts->subsysnqn, ctrl); > > > > > > Found by script or inspection? > > > > > > If by script, it seems unlikely there's only 1 instance > > > where an address of an automatic pointer type is used > > > incorrectly. > > > > Script. But it's using a pretty specific heuristic where we kmalloc a > > pointer and then pass the address. It prints few warnings. Probably > > 40% false positives, but the remaining examples of course are 100% false > > positives. > > I tried anything that looks like a print, ie has a format string argument, > and was taking the address of a local variable as another argument. But > there are lots of weird format designators in the kernel that Coccinelle > doesn't know about for which passing the address of a local variable is > reasonable. So for the moment, there are, as far as I can see, just a lot > of false positives. I did add improving the support for format strings to > my TODO list. fyi: A message from Rasmus awhile ago on the smatch list sent to me and Dan that's relevant. (AFAIK: this list isn't archived anywhere) On Wed, 2015-02-11 at 11:34 +0100, Rasmus Villemoes wrote: > Hi, > > As mentioned, I've been working on getting smatch to do type checking of > the various %p format extensions. The code is now on github > (https://github.com/Villemoes/smatch). > > Note that this work revealed a bug in sparse's handling of string > literals coming from macro expansions > (http://thread.gmane.org/gmane.comp.parsers.sparse/4080). I've applied > one of the suggested fixes, but it's still not clear to me what the > final fix will be in sparse upstream. Anyway, this was good enough to > get the ball rolling. > > While developing this, I found it useful to only enable that specific > check (both to get smatch run faster and to get less noise in the > output), so there's also a few unrelated patches in the printf branch > implementing that feature. > > sparse currently ignores attribute((format)), so the list of printf functions > has been extracted with a perl script and hard-coded. Even if sparse > understood attribute((format)), I wouldn't know how to set up a hook for > 'call of function with this or that attribute'. > > I don't think it's ready to be merged upstream (and whether that will > even happen is of course entirely up to Dan), but now it's out there for > people to play with. I have already sent patches for the four %p bugs > found, but there may be a few more lurking in arch// - I don't > know how to pursuade the build system to go there. > > Rasmus