From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754518AbcK1JAa (ORCPT ); Mon, 28 Nov 2016 04:00:30 -0500 Received: from mail2-relais-roc.national.inria.fr ([192.134.164.83]:42426 "EHLO mail2-relais-roc.national.inria.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750949AbcK1JAX (ORCPT ); Mon, 28 Nov 2016 04:00:23 -0500 X-IronPort-AV: E=Sophos;i="5.31,563,1473112800"; d="scan'208";a="246983803" Date: Mon, 28 Nov 2016 09:59:52 +0100 (CET) From: Julia Lawall X-X-Sender: jll@hadrien To: Fengguang Wu cc: Greg Kroah-Hartman , kbuild-all@01.org, linux-kernel@vger.kernel.org Subject: Re: drivers/staging/greybus/bootrom.c:298:35-39: ERROR: fw is NULL but dereferenced. In-Reply-To: <20161128082458.pe7wfmehxo5rpyxc@wfg-t540p.sh.intel.com> Message-ID: References: <201611270743.Rc4TmNre%fengguang.wu@intel.com> <20161127110633.GA16452@kroah.com> <20161128082458.pe7wfmehxo5rpyxc@wfg-t540p.sh.intel.com> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 28 Nov 2016, Fengguang Wu wrote: > On Sun, Nov 27, 2016 at 12:06:33PM +0100, Greg KH wrote: > > On Sun, Nov 27, 2016 at 07:11:46AM +0800, kbuild test robot wrote: > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > > > master > > > head: a0d60e62ea5c88a9823410e9d0929a513e29dea2 > > > commit: f44dd184634d401f5cf88a6d8b4a60d5ff4f417f Merge greybus driver tree > > > into 4.8-rc6 > > > date: 10 weeks ago > > > > This is a false-positive, sorry. > > Yes it's a false warning: ret != 0, so fw->size in the if test won't be > dereferenced. > Relevant code: > > if (!fw) { > dev_err(dev, "%s: firmware not available\n", __func__); > ret = -EINVAL; > goto unlock; > } > ... > unlock: > mutex_unlock(&bootrom->mutex); > > queue_work: > /* Refresh timeout */ > if (!ret && (offset + size == fw->size)) > next_request = NEXT_REQ_READY_TO_BOOT; > else > next_request = NEXT_REQ_GET_FIRMWARE; > > CC Julia for possible improvements to the coccinelle script. If it's > not convenient to fix there I'll teach the robot to ignore this > particular false warning. This looks like it depends on analyzing the values of variables. It may be best to teach the robot to avoid this warning. I can check these reports if needed for this problem. julia > > Thanks, > Fengguang > > > > coccinelle warnings: (new ones prefixed by >>) > > > > > > >> drivers/staging/greybus/bootrom.c:298:35-39: ERROR: fw is NULL but > > > dereferenced. > > > > > > vim +298 drivers/staging/greybus/bootrom.c > > > > > > 90f1b617 drivers/staging/greybus/firmware.c Viresh Kumar 2015-08-12 282 > > > dev_err(dev, "%s: error allocating response\n", __func__); > > > a956d939 drivers/staging/greybus/bootrom.c Viresh Kumar 2016-05-09 283 > > > ret = -ENOMEM; > > > a956d939 drivers/staging/greybus/bootrom.c Viresh Kumar 2016-05-09 284 > > > goto unlock; > > > 90f1b617 drivers/staging/greybus/firmware.c Viresh Kumar 2015-08-12 285 > > > } > > > 90f1b617 drivers/staging/greybus/firmware.c Viresh Kumar 2015-08-12 286 > > > 90f1b617 drivers/staging/greybus/firmware.c Viresh Kumar 2015-08-12 287 > > > firmware_response = op->response->payload; > > > 98645a9c drivers/staging/greybus/firmware.c Johan Hovold 2015-11-19 288 > > > memcpy(firmware_response->data, fw->data + offset, size); > > > 90f1b617 drivers/staging/greybus/firmware.c Viresh Kumar 2015-08-12 289 > > > fc41c2da drivers/staging/greybus/firmware.c Eli Sennesh 2016-01-08 290 > > > dev_dbg(dev, "responding with firmware (offs = %u, size = %u)\n", offset, > > > fc41c2da drivers/staging/greybus/firmware.c Eli Sennesh 2016-01-08 291 > > > size); > > > fc41c2da drivers/staging/greybus/firmware.c Eli Sennesh 2016-01-08 292 > > > a956d939 drivers/staging/greybus/bootrom.c Viresh Kumar 2016-05-09 293 > > > unlock: > > > a956d939 drivers/staging/greybus/bootrom.c Viresh Kumar 2016-05-09 294 > > > mutex_unlock(&bootrom->mutex); > > > a956d939 drivers/staging/greybus/bootrom.c Viresh Kumar 2016-05-09 295 > > > a956d939 drivers/staging/greybus/bootrom.c Viresh Kumar 2016-05-09 296 > > > queue_work: > > > a956d939 drivers/staging/greybus/bootrom.c Viresh Kumar 2016-05-09 297 > > > /* Refresh timeout */ > > > a4293e1d drivers/staging/greybus/bootrom.c Viresh Kumar 2016-06-22 @298 > > > if (!ret && (offset + size == fw->size)) > > > a4293e1d drivers/staging/greybus/bootrom.c Viresh Kumar 2016-06-22 299 > > > next_request = NEXT_REQ_READY_TO_BOOT; > > > a4293e1d drivers/staging/greybus/bootrom.c Viresh Kumar 2016-06-22 300 > > > else > > > a4293e1d drivers/staging/greybus/bootrom.c Viresh Kumar 2016-06-22 301 > > > next_request = NEXT_REQ_GET_FIRMWARE; > > > a4293e1d drivers/staging/greybus/bootrom.c Viresh Kumar 2016-06-22 302 > > > dbb8cfeb drivers/staging/greybus/bootrom.c Viresh Kumar 2016-06-22 303 > > > gb_bootrom_set_timeout(bootrom, next_request, NEXT_REQ_TIMEOUT_MS); > > > a956d939 drivers/staging/greybus/bootrom.c Viresh Kumar 2016-05-09 304 > > > a956d939 drivers/staging/greybus/bootrom.c Viresh Kumar 2016-05-09 305 > > > return ret; > > > 90f1b617 drivers/staging/greybus/firmware.c Viresh Kumar 2015-08-12 306 > > > } > > > > > > :::::: The code at line 298 was first introduced by commit > > > :::::: a4293e1d4e6416477976ee3bd248589d3fc4bb19 greybus: bootrom: Enhance > > > timeout error message > > > > > > :::::: TO: Viresh Kumar > > > :::::: CC: Greg Kroah-Hartman > > > > > > --- > > > 0-DAY kernel test infrastructure Open Source Technology > > > Center > > > https://lists.01.org/pipermail/kbuild-all Intel > > > Corporation >