From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <yuanhan.liu@linux.intel.com>
Received: from mga05.intel.com (mga05.intel.com [192.55.52.43])
 by dpdk.org (Postfix) with ESMTP id 237711396
 for <dev@dpdk.org>; Thu, 16 Mar 2017 08:51:58 +0100 (CET)
Received: from fmsmga003.fm.intel.com ([10.253.24.29])
 by fmsmga105.fm.intel.com with ESMTP; 16 Mar 2017 00:51:58 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.36,170,1486454400"; d="scan'208";a="835242711"
Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.67.162])
 by FMSMGA003.fm.intel.com with ESMTP; 16 Mar 2017 00:51:56 -0700
Date: Thu, 16 Mar 2017 15:50:14 +0800
From: Yuanhan Liu <yuanhan.liu@linux.intel.com>
To: Maxime Coquelin <maxime.coquelin@redhat.com>
Cc: dev@dpdk.org, Harris James R <james.r.harris@intel.com>,
 Liu Changpeng <changpeng.liu@intel.com>
Message-ID: <20170316075014.GS18844@yliu-dev.sh.intel.com>
References: <1488534682-3494-1-git-send-email-yuanhan.liu@linux.intel.com>
 <1488534682-3494-5-git-send-email-yuanhan.liu@linux.intel.com>
 <979cc67e-b017-085d-27a0-33658c26acca@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <979cc67e-b017-085d-27a0-33658c26acca@redhat.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Subject: Re: [dpdk-dev] [PATCH 04/17] vhost: make notify ops per vhost driver
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Thu, 16 Mar 2017 07:51:59 -0000

On Tue, Mar 14, 2017 at 11:55:20AM +0100, Maxime Coquelin wrote:
> >diff --git a/examples/tep_termination/main.c b/examples/tep_termination/main.c
> >index 8c45128..03c0fbe 100644
> >--- a/examples/tep_termination/main.c
> >+++ b/examples/tep_termination/main.c
> >@@ -1258,7 +1258,8 @@ static inline void __attribute__((always_inline))
> > 	rte_vhost_driver_disable_features(dev_basename,
> > 		1ULL << VIRTIO_NET_F_MRG_RXBUF);
> >
> >-	rte_vhost_driver_callback_register(&virtio_net_device_ops);
> >+	rte_vhost_driver_callback_register(dev_basename,
> >+		&virtio_net_device_ops);
> 
> Return should be checked here, as this function can now return -1.

Right.

> >+struct virtio_net_device_ops const *
> >+vhost_driver_callback_get(const char *path)
> >+{
> >+	struct vhost_user_socket *vsocket;
> >+
> >+	pthread_mutex_lock(&vhost_user.mutex);
> >+	vsocket = find_vhost_user_socket(path);
> >+	pthread_mutex_unlock(&vhost_user.mutex);
> >+
> >+	return vsocket->notify_ops;
> 
> There should be a check against vsocket to avoid NULL pointer
> dereferencing.

Yes.

> >@@ -952,6 +952,9 @@
> > 	if (dev == NULL)
> > 		return -1;
> >
> >+	if (!dev->notify_ops)
> >+		dev->notify_ops = vhost_driver_callback_get(dev->ifname);
> 
> Once vhost_driver_callback_get() fixed, notify_ops can be NULL, and it
> seems to be dereferenced without being checked later on.

Yes.

	--yliu