From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id AE980A0096 for ; Mon, 8 Apr 2019 05:31:43 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 5ACF54C8F; Mon, 8 Apr 2019 05:31:42 +0200 (CEST) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 11C163421; Mon, 8 Apr 2019 05:31:39 +0200 (CEST) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Apr 2019 20:31:38 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,323,1549958400"; d="scan'208";a="147374923" Received: from dpdk-tbie.sh.intel.com ([10.67.104.173]) by FMSMGA003.fm.intel.com with ESMTP; 07 Apr 2019 20:31:37 -0700 Date: Mon, 8 Apr 2019 11:31:11 +0800 From: Tiwei Bie To: Bruce Richardson Cc: dev@dpdk.org, stable@dpdk.org, Maxime Coquelin , Zhihong Wang Message-ID: <20190408033110.GB20719@dpdk-tbie.sh.intel.com> References: <20190405134511.49066-1-bruce.richardson@intel.com> <20190405143709.50352-1-bruce.richardson@intel.com> <20190405143709.50352-3-bruce.richardson@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline In-Reply-To: <20190405143709.50352-3-bruce.richardson@intel.com> User-Agent: Mutt/1.9.4 (2018-02-28) Subject: Re: [dpdk-dev] [PATCH v2 2/5] examples/vhost_scsi: fix missing NULL-check for parameter X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190408033111.4tW4Fg0En_ndBKwbH-BeUSpoJ-9LCpg-ZA8i2v-yNlU@z> On Fri, Apr 05, 2019 at 03:37:06PM +0100, Bruce Richardson wrote: > Coverity points out that there is a check in the main thread loop for the > ctrlr->bdev being NULL, but by that stage the pointer has already been > dereferenced. Therefore, for safety, before we enter the loop do an > initial check on the parameter structure. > > Coverity issue: 158657 > Fixes: db75c7af19bb ("examples/vhost_scsi: introduce a new sample app") > CC: stable@dpdk.org > CC: Maxime Coquelin > CC: Tiwei Bie > CC: Zhihong Wang > > Signed-off-by: Bruce Richardson > --- > examples/vhost_scsi/vhost_scsi.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/examples/vhost_scsi/vhost_scsi.c b/examples/vhost_scsi/vhost_scsi.c > index 2908ff68b..a6465d089 100644 > --- a/examples/vhost_scsi/vhost_scsi.c > +++ b/examples/vhost_scsi/vhost_scsi.c > @@ -285,6 +285,12 @@ ctrlr_worker(void *arg) > cpu_set_t cpuset; > pthread_t thread; > > + if (ctrlr == NULL || ctrlr->bdev == NULL) { > + fprintf(stdout, "%s: Error, invalid argument passed to worker thread\n", Might be better to print to stderr. > + __func__); > + return NULL; Might be better to exit() directly like what the other error handling in this thread does: https://github.com/DPDK/dpdk/blob/bdcfcceb7a0b7534a0dba669279d18bd0f98d5e5/examples/vhost_scsi/vhost_scsi.c#L296-L299 Otherwise destroy_device() will hang on sem_wait(&exit_sem): https://github.com/DPDK/dpdk/blob/bdcfcceb7a0b7534a0dba669279d18bd0f98d5e5/examples/vhost_scsi/vhost_scsi.c#L390 Thanks, Tiwei > + } > + > thread = pthread_self(); > CPU_ZERO(&cpuset); > CPU_SET(0, &cpuset); > -- > 2.20.1 >