Skip to content

Commit 9749fed

Browse files
shemmingerdavem330
shemminger
authored and
davem330
committed
netvsc: use ERR_PTR to avoid dereference issues
The rndis_filter_device_add function is called both in probe context and RTNL context,and creates the netvsc_device inner structure. It is easier to get the RTNL lock annotation correct if it returns the object directly, rather than implicitly by updating network device private data. Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent ea383bf commit 9749fed

File tree

4 files changed

+49
-49
lines changed

4 files changed

+49
-49
lines changed

‎drivers/net/hyperv/hyperv_net.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,8 @@ struct rndis_device {
183183
/* Interface */
184184
struct rndis_message;
185185
struct netvsc_device;
186-
int netvsc_device_add(struct hv_device *device,
187-
const struct netvsc_device_info *info);
186+
struct netvsc_device *netvsc_device_add(struct hv_device *device,
187+
const struct netvsc_device_info *info);
188188
void netvsc_device_remove(struct hv_device *device);
189189
int netvsc_send(struct hv_device *device,
190190
struct hv_netvsc_packet *packet,
@@ -203,8 +203,8 @@ int netvsc_poll(struct napi_struct *napi, int budget);
203203
bool rndis_filter_opened(const struct netvsc_device *nvdev);
204204
int rndis_filter_open(struct netvsc_device *nvdev);
205205
int rndis_filter_close(struct netvsc_device *nvdev);
206-
int rndis_filter_device_add(struct hv_device *dev,
207-
struct netvsc_device_info *info);
206+
struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
207+
struct netvsc_device_info *info);
208208
void rndis_filter_update(struct netvsc_device *nvdev);
209209
void rndis_filter_device_remove(struct hv_device *dev,
210210
struct netvsc_device *nvdev);

‎drivers/net/hyperv/netvsc.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
#include <linux/netdevice.h>
3030
#include <linux/if_ether.h>
3131
#include <linux/vmalloc.h>
32+
#include <linux/rtnetlink.h>
33+
3234
#include <asm/sync_bitops.h>
3335

3436
#include "hyperv_net.h"
@@ -1272,8 +1274,8 @@ void netvsc_channel_cb(void *context)
12721274
* netvsc_device_add - Callback when the device belonging to this
12731275
* driver is added
12741276
*/
1275-
int netvsc_device_add(struct hv_device *device,
1276-
const struct netvsc_device_info *device_info)
1277+
struct netvsc_device *netvsc_device_add(struct hv_device *device,
1278+
const struct netvsc_device_info *device_info)
12771279
{
12781280
int i, ret = 0;
12791281
int ring_size = device_info->ring_size;
@@ -1283,7 +1285,7 @@ int netvsc_device_add(struct hv_device *device,
12831285

12841286
net_device = alloc_net_device();
12851287
if (!net_device)
1286-
return -ENOMEM;
1288+
return ERR_PTR(-ENOMEM);
12871289

12881290
net_device->ring_size = ring_size;
12891291

@@ -1339,7 +1341,7 @@ int netvsc_device_add(struct hv_device *device,
13391341
goto close;
13401342
}
13411343

1342-
return ret;
1344+
return net_device;
13431345

13441346
close:
13451347
netif_napi_del(&net_device->chan_table[0].napi);
@@ -1350,6 +1352,5 @@ int netvsc_device_add(struct hv_device *device,
13501352
cleanup:
13511353
free_netvsc_device(&net_device->rcu);
13521354

1353-
return ret;
1354-
1355+
return ERR_PTR(ret);
13551356
}

‎drivers/net/hyperv/netvsc_drv.c

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -717,6 +717,7 @@ static int netvsc_set_queues(struct net_device *net, struct hv_device *dev,
717717
u32 num_chn)
718718
{
719719
struct netvsc_device_info device_info;
720+
struct netvsc_device *net_device;
720721
int ret;
721722

722723
memset(&device_info, 0, sizeof(device_info));
@@ -732,7 +733,8 @@ static int netvsc_set_queues(struct net_device *net, struct hv_device *dev,
732733
if (ret)
733734
return ret;
734735

735-
return rndis_filter_device_add(dev, &device_info);
736+
net_device = rndis_filter_device_add(dev, &device_info);
737+
return IS_ERR(net_device) ? PTR_ERR(net_device) : 0;
736738
}
737739

738740
static int netvsc_set_channels(struct net_device *net,
@@ -845,8 +847,10 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
845847
struct net_device_context *ndevctx = netdev_priv(ndev);
846848
struct netvsc_device *nvdev = rtnl_dereference(ndevctx->nvdev);
847849
struct hv_device *hdev = ndevctx->device_ctx;
850+
int orig_mtu = ndev->mtu;
848851
struct netvsc_device_info device_info;
849852
bool was_opened;
853+
int ret = 0;
850854

851855
if (!nvdev || nvdev->destroy)
852856
return -ENODEV;
@@ -863,16 +867,16 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
863867

864868
rndis_filter_device_remove(hdev, nvdev);
865869

866-
/* 'nvdev' has been freed in rndis_filter_device_remove() ->
867-
* netvsc_device_remove () -> free_netvsc_device().
868-
* We mustn't access it before it's re-created in
869-
* rndis_filter_device_add() -> netvsc_device_add().
870-
*/
871-
872870
ndev->mtu = mtu;
873871

874-
rndis_filter_device_add(hdev, &device_info);
875-
nvdev = rtnl_dereference(ndevctx->nvdev);
872+
nvdev = rndis_filter_device_add(hdev, &device_info);
873+
if (IS_ERR(nvdev)) {
874+
ret = PTR_ERR(nvdev);
875+
876+
/* Attempt rollback to original MTU */
877+
ndev->mtu = orig_mtu;
878+
rndis_filter_device_add(hdev, &device_info);
879+
}
876880

877881
if (was_opened)
878882
rndis_filter_open(nvdev);
@@ -882,7 +886,7 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
882886
/* We may have missed link change notifications */
883887
schedule_delayed_work(&ndevctx->dwork, 0);
884888

885-
return 0;
889+
return ret;
886890
}
887891

888892
static void netvsc_get_stats64(struct net_device *net,
@@ -1525,8 +1529,10 @@ static int netvsc_probe(struct hv_device *dev,
15251529
memset(&device_info, 0, sizeof(device_info));
15261530
device_info.ring_size = ring_size;
15271531
device_info.num_chn = VRSS_CHANNEL_DEFAULT;
1528-
ret = rndis_filter_device_add(dev, &device_info);
1529-
if (ret != 0) {
1532+
1533+
nvdev = rndis_filter_device_add(dev, &device_info);
1534+
if (IS_ERR(nvdev)) {
1535+
ret = PTR_ERR(nvdev);
15301536
netdev_err(net, "unable to add netvsc device (ret %d)\n", ret);
15311537
free_netdev(net);
15321538
hv_set_drvdata(dev, NULL);
@@ -1540,11 +1546,11 @@ static int netvsc_probe(struct hv_device *dev,
15401546
NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX;
15411547
net->vlan_features = net->features;
15421548

1543-
/* RCU not necessary here, device not registered */
1544-
nvdev = net_device_ctx->nvdev;
15451549
netif_set_real_num_tx_queues(net, nvdev->num_chn);
15461550
netif_set_real_num_rx_queues(net, nvdev->num_chn);
15471551

1552+
netdev_lockdep_set_classes(net);
1553+
15481554
/* MTU range: 68 - 1500 or 65521 */
15491555
net->min_mtu = NETVSC_MTU_MIN;
15501556
if (nvdev->nvsp_version >= NVSP_PROTOCOL_VERSION_2)

‎drivers/net/hyperv/rndis_filter.c

Lines changed: 18 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -658,9 +658,9 @@ int rndis_filter_set_device_mac(struct net_device *ndev, char *mac)
658658

659659
static int
660660
rndis_filter_set_offload_params(struct net_device *ndev,
661+
struct netvsc_device *nvdev,
661662
struct ndis_offload_params *req_offloads)
662663
{
663-
struct netvsc_device *nvdev = net_device_to_netvsc_device(ndev);
664664
struct rndis_device *rdev = nvdev->extension;
665665
struct rndis_request *request;
666666
struct rndis_set_request *set;
@@ -1052,8 +1052,8 @@ static void netvsc_sc_open(struct vmbus_channel *new_sc)
10521052
complete(&nvscdev->channel_init_wait);
10531053
}
10541054

1055-
int rndis_filter_device_add(struct hv_device *dev,
1056-
struct netvsc_device_info *device_info)
1055+
struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
1056+
struct netvsc_device_info *device_info)
10571057
{
10581058
struct net_device *net = hv_get_drvdata(dev);
10591059
struct net_device_context *net_device_ctx = netdev_priv(net);
@@ -1072,21 +1072,20 @@ int rndis_filter_device_add(struct hv_device *dev,
10721072

10731073
rndis_device = get_rndis_device();
10741074
if (!rndis_device)
1075-
return -ENODEV;
1075+
return ERR_PTR(-ENODEV);
10761076

10771077
/*
10781078
* Let the inner driver handle this first to create the netvsc channel
10791079
* NOTE! Once the channel is created, we may get a receive callback
10801080
* (RndisFilterOnReceive()) before this call is completed
10811081
*/
1082-
ret = netvsc_device_add(dev, device_info);
1083-
if (ret != 0) {
1082+
net_device = netvsc_device_add(dev, device_info);
1083+
if (IS_ERR(net_device)) {
10841084
kfree(rndis_device);
1085-
return ret;
1085+
return net_device;
10861086
}
10871087

10881088
/* Initialize the rndis device */
1089-
net_device = net_device_ctx->nvdev;
10901089
net_device->max_chn = 1;
10911090
net_device->num_chn = 1;
10921091

@@ -1097,10 +1096,8 @@ int rndis_filter_device_add(struct hv_device *dev,
10971096

10981097
/* Send the rndis initialization message */
10991098
ret = rndis_filter_init_device(rndis_device);
1100-
if (ret != 0) {
1101-
rndis_filter_device_remove(dev, net_device);
1102-
return ret;
1103-
}
1099+
if (ret != 0)
1100+
goto err_dev_remv;
11041101

11051102
/* Get the MTU from the host */
11061103
size = sizeof(u32);
@@ -1112,19 +1109,15 @@ int rndis_filter_device_add(struct hv_device *dev,
11121109

11131110
/* Get the mac address */
11141111
ret = rndis_filter_query_device_mac(rndis_device);
1115-
if (ret != 0) {
1116-
rndis_filter_device_remove(dev, net_device);
1117-
return ret;
1118-
}
1112+
if (ret != 0)
1113+
goto err_dev_remv;
11191114

11201115
memcpy(device_info->mac_adr, rndis_device->hw_mac_adr, ETH_ALEN);
11211116

11221117
/* Find HW offload capabilities */
11231118
ret = rndis_query_hwcaps(rndis_device, &hwcaps);
1124-
if (ret != 0) {
1125-
rndis_filter_device_remove(dev, net_device);
1126-
return ret;
1127-
}
1119+
if (ret != 0)
1120+
goto err_dev_remv;
11281121

11291122
/* A value of zero means "no change"; now turn on what we want. */
11301123
memset(&offloads, 0, sizeof(struct ndis_offload_params));
@@ -1179,7 +1172,7 @@ int rndis_filter_device_add(struct hv_device *dev,
11791172

11801173
netif_set_gso_max_size(net, gso_max_size);
11811174

1182-
ret = rndis_filter_set_offload_params(net, &offloads);
1175+
ret = rndis_filter_set_offload_params(net, net_device, &offloads);
11831176
if (ret)
11841177
goto err_dev_remv;
11851178

@@ -1190,7 +1183,7 @@ int rndis_filter_device_add(struct hv_device *dev,
11901183
rndis_device->link_state ? "down" : "up");
11911184

11921185
if (net_device->nvsp_version < NVSP_PROTOCOL_VERSION_5)
1193-
return 0;
1186+
return net_device;
11941187

11951188
rndis_filter_query_link_speed(rndis_device);
11961189

@@ -1223,7 +1216,7 @@ int rndis_filter_device_add(struct hv_device *dev,
12231216

12241217
num_rss_qs = net_device->num_chn - 1;
12251218
if (num_rss_qs == 0)
1226-
return 0;
1219+
return net_device;
12271220

12281221
refcount_set(&net_device->sc_offered, num_rss_qs);
12291222
vmbus_set_sc_create_callback(dev->channel, netvsc_sc_open);
@@ -1260,11 +1253,11 @@ int rndis_filter_device_add(struct hv_device *dev,
12601253
net_device->num_chn = 1;
12611254
}
12621255

1263-
return 0; /* return 0 because primary channel can be used alone */
1256+
return net_device;
12641257

12651258
err_dev_remv:
12661259
rndis_filter_device_remove(dev, net_device);
1267-
return ret;
1260+
return ERR_PTR(ret);
12681261
}
12691262

12701263
void rndis_filter_device_remove(struct hv_device *dev,

0 commit comments

Comments
 (0)