From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <bruce.richardson@intel.com>
Received: from mga02.intel.com (mga02.intel.com [134.134.136.20])
by dpdk.org (Postfix) with ESMTP id 4B3E74CA6;
Mon, 8 Apr 2019 11:37:12 +0200 (CEST)
X-Amp-Result: UNSCANNABLE
X-Amp-File-Uploaded: False
Received: from orsmga004.jf.intel.com ([10.7.209.38])
by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
08 Apr 2019 02:37:11 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.60,324,1549958400"; d="scan'208";a="289666384"
Received: from bricha3-mobl.ger.corp.intel.com ([10.237.221.35])
by orsmga004.jf.intel.com with SMTP; 08 Apr 2019 02:37:08 -0700
Received: by (sSMTP sendmail emulation); Mon, 08 Apr 2019 10:37:08 +0100
Date: Mon, 8 Apr 2019 10:37:07 +0100
From: Bruce Richardson <bruce.richardson@intel.com>
To: Tiwei Bie <tiwei.bie@intel.com>
Cc: dev@dpdk.org, stable@dpdk.org,
Maxime Coquelin <maxime.coquelin@redhat.com>,
Zhihong Wang <zhihong.wang@intel.com>
Message-ID: <20190408093707.GA1734@bricha3-MOBL.ger.corp.intel.com>
References: <20190405134511.49066-1-bruce.richardson@intel.com>
<20190405143709.50352-1-bruce.richardson@intel.com>
<20190405143709.50352-3-bruce.richardson@intel.com>
<20190408033110.GB20719@dpdk-tbie.sh.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190408033110.GB20719@dpdk-tbie.sh.intel.com>
User-Agent: Mutt/1.11.4 (2019-03-13)
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Mon, 08 Apr 2019 09:37:13 -0000
On Mon, Apr 08, 2019 at 11:31:11AM +0800, Tiwei Bie wrote:
> 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 <maxime.coquelin@redhat.com>
> > CC: Tiwei Bie <tiwei.bie@intel.com>
> > CC: Zhihong Wang <zhihong.wang@intel.com>
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> > 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.
>
Yes, this looks a typo on my part.
> > + __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
>
Ok, will change in next revision
From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by dpdk.space (Postfix) with ESMTP id 8AA73A0096
for <public@inbox.dpdk.org>; Mon, 8 Apr 2019 11:37:14 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id 27D662C24;
Mon, 8 Apr 2019 11:37:14 +0200 (CEST)
Received: from mga02.intel.com (mga02.intel.com [134.134.136.20])
by dpdk.org (Postfix) with ESMTP id 4B3E74CA6;
Mon, 8 Apr 2019 11:37:12 +0200 (CEST)
X-Amp-Result: UNSCANNABLE
X-Amp-File-Uploaded: False
Received: from orsmga004.jf.intel.com ([10.7.209.38])
by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
08 Apr 2019 02:37:11 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.60,324,1549958400"; d="scan'208";a="289666384"
Received: from bricha3-mobl.ger.corp.intel.com ([10.237.221.35])
by orsmga004.jf.intel.com with SMTP; 08 Apr 2019 02:37:08 -0700
Received: by (sSMTP sendmail emulation); Mon, 08 Apr 2019 10:37:08 +0100
Date: Mon, 8 Apr 2019 10:37:07 +0100
From: Bruce Richardson <bruce.richardson@intel.com>
To: Tiwei Bie <tiwei.bie@intel.com>
Cc: dev@dpdk.org, stable@dpdk.org,
Maxime Coquelin <maxime.coquelin@redhat.com>,
Zhihong Wang <zhihong.wang@intel.com>
Message-ID: <20190408093707.GA1734@bricha3-MOBL.ger.corp.intel.com>
References: <20190405134511.49066-1-bruce.richardson@intel.com>
<20190405143709.50352-1-bruce.richardson@intel.com>
<20190405143709.50352-3-bruce.richardson@intel.com>
<20190408033110.GB20719@dpdk-tbie.sh.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Disposition: inline
In-Reply-To: <20190408033110.GB20719@dpdk-tbie.sh.intel.com>
User-Agent: Mutt/1.11.4 (2019-03-13)
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
Message-ID: <20190408093707.FzgJetohikzeHnyrPLXaN4DlfnEPEOykY60WqZLrn6o@z>
On Mon, Apr 08, 2019 at 11:31:11AM +0800, Tiwei Bie wrote:
> 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 <maxime.coquelin@redhat.com>
> > CC: Tiwei Bie <tiwei.bie@intel.com>
> > CC: Zhihong Wang <zhihong.wang@intel.com>
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> > 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.
>
Yes, this looks a typo on my part.
> > + __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
>
Ok, will change in next revision