On Tue, 3 Jul 2018 15:01:42 +0300
Dan Carpenter <dan.carpenter@xxxxxxxxxx> 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);
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
