Skip to content

Commit cf124db

Browse files
committed
net: Fix inconsistent teardown and release of private netdev state.
Network devices can allocate reasources and private memory using netdev_ops->ndo_init(). However, the release of these resources can occur in one of two different places. Either netdev_ops->ndo_uninit() or netdev->destructor(). The decision of which operation frees the resources depends upon whether it is necessary for all netdev refs to be released before it is safe to perform the freeing. netdev_ops->ndo_uninit() presumably can occur right after the NETDEV_UNREGISTER notifier completes and the unicast and multicast address lists are flushed. netdev->destructor(), on the other hand, does not run until the netdev references all go away. Further complicating the situation is that netdev->destructor() almost universally does also a free_netdev(). This creates a problem for the logic in register_netdevice(). Because all callers of register_netdevice() manage the freeing of the netdev, and invoke free_netdev(dev) if register_netdevice() fails. If netdev_ops->ndo_init() succeeds, but something else fails inside of register_netdevice(), it does call ndo_ops->ndo_uninit(). But it is not able to invoke netdev->destructor(). This is because netdev->destructor() will do a free_netdev() and then the caller of register_netdevice() will do the same. However, this means that the resources that would normally be released by netdev->destructor() will not be. Over the years drivers have added local hacks to deal with this, by invoking their destructor parts by hand when register_netdevice() fails. Many drivers do not try to deal with this, and instead we have leaks. Let's close this hole by formalizing the distinction between what private things need to be freed up by netdev->destructor() and whether the driver needs unregister_netdevice() to perform the free_netdev(). netdev->priv_destructor() performs all actions to free up the private resources that used to be freed by netdev->destructor(), except for free_netdev(). netdev->needs_free_netdev is a boolean that indicates whether free_netdev() should be done at the end of unregister_netdevice(). Now, register_netdevice() can sanely release all resources after ndo_ops->ndo_init() succeeds, by invoking both ndo_ops->ndo_uninit() and netdev->priv_destructor(). And at the end of unregister_netdevice(), we invoke netdev->priv_destructor() and optionally call free_netdev(). Signed-off-by: David S. Miller <[email protected]>
1 parent 7005cad commit cf124db

File tree

62 files changed

+105
-103
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

62 files changed

+105
-103
lines changed

drivers/net/bonding/bond_main.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4192,7 +4192,6 @@ static void bond_destructor(struct net_device *bond_dev)
41924192
struct bonding *bond = netdev_priv(bond_dev);
41934193
if (bond->wq)
41944194
destroy_workqueue(bond->wq);
4195-
free_netdev(bond_dev);
41964195
}
41974196

41984197
void bond_setup(struct net_device *bond_dev)
@@ -4212,7 +4211,8 @@ void bond_setup(struct net_device *bond_dev)
42124211
bond_dev->netdev_ops = &bond_netdev_ops;
42134212
bond_dev->ethtool_ops = &bond_ethtool_ops;
42144213

4215-
bond_dev->destructor = bond_destructor;
4214+
bond_dev->needs_free_netdev = true;
4215+
bond_dev->priv_destructor = bond_destructor;
42164216

42174217
SET_NETDEV_DEVTYPE(bond_dev, &bond_type);
42184218

@@ -4736,7 +4736,7 @@ int bond_create(struct net *net, const char *name)
47364736

47374737
rtnl_unlock();
47384738
if (res < 0)
4739-
bond_destructor(bond_dev);
4739+
free_netdev(bond_dev);
47404740
return res;
47414741
}
47424742

drivers/net/caif/caif_hsi.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1121,7 +1121,7 @@ static void cfhsi_setup(struct net_device *dev)
11211121
dev->flags = IFF_POINTOPOINT | IFF_NOARP;
11221122
dev->mtu = CFHSI_MAX_CAIF_FRAME_SZ;
11231123
dev->priv_flags |= IFF_NO_QUEUE;
1124-
dev->destructor = free_netdev;
1124+
dev->needs_free_netdev = true;
11251125
dev->netdev_ops = &cfhsi_netdevops;
11261126
for (i = 0; i < CFHSI_PRIO_LAST; ++i)
11271127
skb_queue_head_init(&cfhsi->qhead[i]);

drivers/net/caif/caif_serial.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,7 @@ static void caifdev_setup(struct net_device *dev)
428428
dev->flags = IFF_POINTOPOINT | IFF_NOARP;
429429
dev->mtu = CAIF_MAX_MTU;
430430
dev->priv_flags |= IFF_NO_QUEUE;
431-
dev->destructor = free_netdev;
431+
dev->needs_free_netdev = true;
432432
skb_queue_head_init(&serdev->head);
433433
serdev->common.link_select = CAIF_LINK_LOW_LATENCY;
434434
serdev->common.use_frag = true;

drivers/net/caif/caif_spi.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -712,7 +712,7 @@ static void cfspi_setup(struct net_device *dev)
712712
dev->flags = IFF_NOARP | IFF_POINTOPOINT;
713713
dev->priv_flags |= IFF_NO_QUEUE;
714714
dev->mtu = SPI_MAX_PAYLOAD_SIZE;
715-
dev->destructor = free_netdev;
715+
dev->needs_free_netdev = true;
716716
skb_queue_head_init(&cfspi->qhead);
717717
skb_queue_head_init(&cfspi->chead);
718718
cfspi->cfdev.link_select = CAIF_LINK_HIGH_BANDW;

drivers/net/caif/caif_virtio.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -617,7 +617,7 @@ static void cfv_netdev_setup(struct net_device *netdev)
617617
netdev->tx_queue_len = 100;
618618
netdev->flags = IFF_POINTOPOINT | IFF_NOARP;
619619
netdev->mtu = CFV_DEF_MTU_SIZE;
620-
netdev->destructor = free_netdev;
620+
netdev->needs_free_netdev = true;
621621
}
622622

623623
/* Create debugfs counters for the device */

drivers/net/can/slcan.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,7 @@ static int slc_open(struct net_device *dev)
417417
static void slc_free_netdev(struct net_device *dev)
418418
{
419419
int i = dev->base_addr;
420-
free_netdev(dev);
420+
421421
slcan_devs[i] = NULL;
422422
}
423423

@@ -436,7 +436,8 @@ static const struct net_device_ops slc_netdev_ops = {
436436
static void slc_setup(struct net_device *dev)
437437
{
438438
dev->netdev_ops = &slc_netdev_ops;
439-
dev->destructor = slc_free_netdev;
439+
dev->needs_free_netdev = true;
440+
dev->priv_destructor = slc_free_netdev;
440441

441442
dev->hard_header_len = 0;
442443
dev->addr_len = 0;
@@ -761,8 +762,6 @@ static void __exit slcan_exit(void)
761762
if (sl->tty) {
762763
printk(KERN_ERR "%s: tty discipline still running\n",
763764
dev->name);
764-
/* Intentionally leak the control block. */
765-
dev->destructor = NULL;
766765
}
767766

768767
unregister_netdev(dev);

drivers/net/can/vcan.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ static void vcan_setup(struct net_device *dev)
163163
dev->flags |= IFF_ECHO;
164164

165165
dev->netdev_ops = &vcan_netdev_ops;
166-
dev->destructor = free_netdev;
166+
dev->needs_free_netdev = true;
167167
}
168168

169169
static struct rtnl_link_ops vcan_link_ops __read_mostly = {

drivers/net/can/vxcan.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ static void vxcan_setup(struct net_device *dev)
156156
dev->tx_queue_len = 0;
157157
dev->flags = (IFF_NOARP|IFF_ECHO);
158158
dev->netdev_ops = &vxcan_netdev_ops;
159-
dev->destructor = free_netdev;
159+
dev->needs_free_netdev = true;
160160
}
161161

162162
/* forward declaration for rtnl_create_link() */

drivers/net/dummy.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,6 @@ static void dummy_free_netdev(struct net_device *dev)
328328
struct dummy_priv *priv = netdev_priv(dev);
329329

330330
kfree(priv->vfinfo);
331-
free_netdev(dev);
332331
}
333332

334333
static void dummy_setup(struct net_device *dev)
@@ -338,7 +337,8 @@ static void dummy_setup(struct net_device *dev)
338337
/* Initialize the device structure. */
339338
dev->netdev_ops = &dummy_netdev_ops;
340339
dev->ethtool_ops = &dummy_ethtool_ops;
341-
dev->destructor = dummy_free_netdev;
340+
dev->needs_free_netdev = true;
341+
dev->priv_destructor = dummy_free_netdev;
342342

343343
/* Fill in device structure with ethernet-generic values. */
344344
dev->flags |= IFF_NOARP;

drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4525,7 +4525,7 @@ static void dummy_setup(struct net_device *dev)
45254525
/* Initialize the device structure. */
45264526
dev->netdev_ops = &cxgb4_mgmt_netdev_ops;
45274527
dev->ethtool_ops = &cxgb4_mgmt_ethtool_ops;
4528-
dev->destructor = free_netdev;
4528+
dev->needs_free_netdev = true;
45294529
}
45304530

45314531
static int config_mgmt_dev(struct pci_dev *pdev)

0 commit comments

Comments
 (0)