[bug report] netvsc: use ERR_PTR to avoid dereference issues

[Stephen Hemminger]

On Tue, 3 Jul 2018 15:01:42 +0300
Dan Carpenter <dan.carpenter at oracle.com> wrote:

> Hello stephen hemminger,
> 
> The patch 9749fed5d43d: "netvsc: use ERR_PTR to avoid dereference
> issues" from Jul 19, 2017, leads to the following static checker
> warning:
> 
> 	drivers/net/hyperv/rndis_filter.c:1344 rndis_filter_device_add()
> 	warn: passing zero to 'ERR_PTR'
> 
> drivers/net/hyperv/rndis_filter.c
>   1300          ret = rndis_filter_query_device(rndis_device, net_device,
>   1301                                          OID_GEN_RECEIVE_SCALE_CAPABILITIES,
>   1302                                          &rsscap, &rsscap_size);
>   1303          if (ret || rsscap.num_recv_que < 2)
>                            ^^^^^^^^^^^^^^^^^^^^^^^
> I think this used to be a success path, but now it leads to a NULL
> dereference in the caller.  I guess we should set "ret = -ENOMEM;" here?
> 
>   1304                  goto out;
>   1305  
>   1306          /* This guarantees that num_possible_rss_qs <= num_online_cpus */
>   1307          num_possible_rss_qs = min_t(u32, num_online_cpus(),
>   1308                                      rsscap.num_recv_que);
>   1309  
>   1310          net_device->max_chn = min_t(u32, VRSS_CHANNEL_MAX, num_possible_rss_qs);
>   1311  
>   1312          /* We will use the given number of channels if available. */
>   1313          net_device->num_chn = min(net_device->max_chn, device_info->num_chn);
>   1314  
>   1315          for (i = 0; i < ITAB_NUM; i++)
>   1316                  rndis_device->rx_table[i] = ethtool_rxfh_indir_default(
>   1317                                                  i, net_device->num_chn);
>   1318  
>   1319          atomic_set(&net_device->open_chn, 1);
>   1320          vmbus_set_sc_create_callback(dev->channel, netvsc_sc_open);
>   1321  
>   1322          for (i = 1; i < net_device->num_chn; i++) {
>   1323                  ret = netvsc_alloc_recv_comp_ring(net_device, i);
>   1324                  if (ret) {
>   1325                          while (--i != 0)
>   1326                                  vfree(net_device->chan_table[i].mrc.slots);
>   1327                          goto out;
>   1328                  }
>   1329          }
>   1330  
>   1331          for (i = 1; i < net_device->num_chn; i++)
>   1332                  netif_napi_add(net, &net_device->chan_table[i].napi,
>   1333                                 netvsc_poll, NAPI_POLL_WEIGHT);
>   1334  
>   1335          return net_device;
>   1336  
>   1337  out:
>   1338          /* setting up multiple channels failed */
>   1339          net_device->max_chn = 1;
>   1340          net_device->num_chn = 1;
>   1341  
>   1342  err_dev_remv:
>   1343          rndis_filter_device_remove(dev, net_device);
S
>   1344          return ERR_PTR(ret);
> 
> regards,
> dan carpenter

It looks like there is a missing return after out:
all the fall throughs in that path should just work with single queue.
The current code would fail on old versions of Hyper-V.


This should be enough to fix the problem:

diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index 9b4e3c3787e5..2a5209f23f29 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -1338,6 +1338,7 @@ struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
 	/* setting up multiple channels failed */
 	net_device->max_chn = 1;
 	net_device->num_chn = 1;
+	return net_device;
 
 err_dev_remv:
 	rndis_filter_device_remove(dev, net_device);