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: sta...@dpdk.org
> CC: Maxime Coquelin <maxime.coque...@redhat.com>
> CC: Tiwei Bie <tiwei....@intel.com>
> CC: Zhihong Wang <zhihong.w...@intel.com>
>
> Signed-off-by: Bruce Richardson <bruce.richard...@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.
> + __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
>
