On 4/13/2017 10:21 AM, Yuanhan Liu wrote:
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.coque...@redhat.com>
Cc: Yuanhan Liu <yuanhan....@linux.intel.com>

Signed-off-by: Jianfeng Tan <jianfeng....@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.


OK, I'll drop this fix from this patch.

Reply via email to