[dpdk-dev] [PATCH] net/virtio-user: fix cannot get initialized

[Tan, Jianfeng]

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 at redhat.com>
>> Cc: Yuanhan Liu <yuanhan.liu at linux.intel.com>
>>
>> Signed-off-by: Jianfeng Tan <jianfeng.tan at 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.

Thanks,
Jianfeng