From mboxrd@z Thu Jan  1 00:00:00 1970
From: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Subject: Re: [PATCH] net/virtio-user: fix cannot get initialized
Date: Thu, 13 Apr 2017 10:21:28 +0800
Message-ID: <20170413022128.GG7333@yliu-dev.sh.intel.com>
References: <1491551860-15005-1-git-send-email-jianfeng.tan@intel.com>
 <20170413015812.GE7333@yliu-dev.sh.intel.com>
 <5e7b5876-0f98-9c42-965e-e91b318cd17f@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: dev@dpdk.org, Maxime Coquelin <maxime.coquelin@redhat.com>
To: "Tan, Jianfeng" <jianfeng.tan@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mga05.intel.com (mga05.intel.com [192.55.52.43])
 by dpdk.org (Postfix) with ESMTP id A4E0B1E2B
 for <dev@dpdk.org>; Thu, 13 Apr 2017 04:24:25 +0200 (CEST)
Content-Disposition: inline
In-Reply-To: <5e7b5876-0f98-9c42-965e-e91b318cd17f@intel.com>
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>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Thu, Apr 13, 2017 at 10:18:43AM +0800, Tan, Jianfeng wrote:
> 
> 
> On 4/13/2017 9:58 AM, Yuanhan Liu wrote:
> >On Fri, Apr 07, 2017 at 07:57:40AM +0000, Jianfeng Tan wrote:
> >>After the introduction of vhost MTU, VIRTIO_NET_F_MTU is enabled
> >>by default. However, virtio-user vtpci does not support to get
> >>MTU from device yet, i.e., vtpci_read_dev_config(MTU) fails.
> >>Plus, struct virtio_net_config is defined as a uninitialized
> >>variable, and could be different values in
> >>virtio_negotiate_features() and virtio_init_device().
> >>
> >>In some cases, it passes the check in virtio_negotiate_features()
> >>but fails the check in virtio_init_device(). As a result,
> >>virtio-user canno be initialized.
> >>
> >>To fix it, (1) accessing uninitialized variable is not a good
> >>practice, so initialize it as zero; (2) explicitly disable MTU
> >>feature in virtio-user.
> >>
> >>Fixes: 49d26d9e3f47 ("net/virtio: support MTU feature")
> >>Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> >>Cc: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> >>
> >>Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> >>---
> >>  drivers/net/virtio/virtio_ethdev.c               | 4 ++--
> >>  drivers/net/virtio/virtio_user/virtio_user_dev.c | 6 ++++++
> >>  2 files changed, 8 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> >>index 78cb3e8..4c43784 100644
> >>--- a/drivers/net/virtio/virtio_ethdev.c
> >>+++ b/drivers/net/virtio/virtio_ethdev.c
> >>@@ -1163,7 +1163,7 @@ virtio_negotiate_features(struct virtio_hw *hw, uint64_t req_features)
> >>  	/* If supported, ensure MTU value is valid before acknowledging it. */
> >>  	if (host_features & req_features & (1ULL << VIRTIO_NET_F_MTU)) {
> >>-		struct virtio_net_config config;
> >>+		struct virtio_net_config config = {0};
> >virtio-user does not support the MTU feature, this patch should not be
> >reached. The virtio-user feature negotiation should be broken.
> >
> >	--yliu
> >
> 
> Yes, it will not come to here anyway. But some static code analyzer might
> report this as an error: there's chance to read uninitialized variable.

If so, fix it in another patch, with the detailed (and real) errors in
the commit log.

	--yliu