From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1034965AbdEWWYl (ORCPT ); Tue, 23 May 2017 18:24:41 -0400 Received: from mga07.intel.com ([134.134.136.100]:44568 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1034944AbdEWWYg (ORCPT ); Tue, 23 May 2017 18:24:36 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.38,383,1491289200"; d="scan'208";a="105783900" Message-ID: <1495578269.14443.4.camel@linux.intel.com> Subject: Re: [PATCH v2 1/5] HID: intel_ish-hid: fix potential uninitialized data usage From: Srinivas Pandruvada To: Arnd Bergmann , Jiri Kosina Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org Date: Tue, 23 May 2017 15:24:29 -0700 In-Reply-To: <20170518202144.3482304-2-arnd@arndb.de> References: <20170518202144.3482304-1-arnd@arndb.de> <20170518202144.3482304-2-arnd@arndb.de> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.22.4 (3.22.4-2.fc25) Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 2017-05-18 at 22:21 +0200, Arnd Bergmann wrote: > gcc points out an uninialized pointer dereference that could happen > if we ever get to recv_ishtp_cl_msg_dma() or recv_ishtp_cl_msg() > with an empty &dev->read_list: In that case complete_rb should be NULL and it should not go to if (complete_rb) { shouldn't enter and cl is not dereferenced. So not sure why is this warning. Thanks, Srinivas > > drivers/hid/intel-ish-hid/ishtp/client.c: In function > 'recv_ishtp_cl_msg_dma': > drivers/hid/intel-ish-hid/ishtp/client.c:1049:3: error: 'cl' may be > used uninitialized in this function [-Werror=maybe-uninitialized] > > The warning only appeared in very few randconfig builds, as the > spinlocks tend to prevent gcc from tracing the variables. I only > saw it in configurations that had neither SMP nor LOCKDEP enabled. > > As we can see, we only enter the case if 'complete_rb' is non-NULL, > and then 'cl' is known to point to complete_rb->cl. Adding another > initialization to the same pointer is harmless here and makes it > clear to the compiler that the behavior is well-defined. > > Signed-off-by: Arnd Bergmann > --- >  drivers/hid/intel-ish-hid/ishtp/client.c | 2 ++ >  1 file changed, 2 insertions(+) > > diff --git a/drivers/hid/intel-ish-hid/ishtp/client.c > b/drivers/hid/intel-ish-hid/ishtp/client.c > index aad61328f282..78d393e616a4 100644 > --- a/drivers/hid/intel-ish-hid/ishtp/client.c > +++ b/drivers/hid/intel-ish-hid/ishtp/client.c > @@ -925,6 +925,7 @@ void recv_ishtp_cl_msg(struct ishtp_device *dev, >   } >   >   if (complete_rb) { > + cl = complete_rb->cl; >   getnstimeofday(&cl->ts_rx); >   ++cl->recv_msg_cnt_ipc; >   ishtp_cl_read_complete(complete_rb); > @@ -1045,6 +1046,7 @@ void recv_ishtp_cl_msg_dma(struct ishtp_device > *dev, void *msg, >   } >   >   if (complete_rb) { > + cl = complete_rb->cl; >   getnstimeofday(&cl->ts_rx); >   ++cl->recv_msg_cnt_dma; >   ishtp_cl_read_complete(complete_rb);