Commit af301517 authored by Patrick McHardy's avatar Patrick McHardy Committed by David S. Miller

[VLAN]: Simplify vlan unregistration

Keep track of the number of VLAN devices in a vlan group. This allows
to have the caller sense when the group is going to be destroyed and
stop using it, which in turn allows to remove the wrapper around
unregister_vlan_dev for the NETDEV_UNREGISTER notifier and avoid
iterating over all possible VLAN ids whenever a device in unregistered.

Also fix what looks like a use-after-free (but is actually safe since
we're holding the RTNL), the real_dev reference should not be dropped
while we still use it.
Signed-off-by: default avatarPatrick McHardy <kaber@trash.net>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent acc5efbc
...@@ -82,6 +82,7 @@ extern void vlan_ioctl_set(int (*hook)(struct net *, void __user *)); ...@@ -82,6 +82,7 @@ extern void vlan_ioctl_set(int (*hook)(struct net *, void __user *));
struct vlan_group { struct vlan_group {
int real_dev_ifindex; /* The ifindex of the ethernet(like) device the vlan is attached to. */ int real_dev_ifindex; /* The ifindex of the ethernet(like) device the vlan is attached to. */
unsigned int nr_vlans;
struct hlist_node hlist; /* linked list */ struct hlist_node hlist; /* linked list */
struct net_device **vlan_devices_arrays[VLAN_GROUP_ARRAY_SPLIT_PARTS]; struct net_device **vlan_devices_arrays[VLAN_GROUP_ARRAY_SPLIT_PARTS];
struct rcu_head rcu; struct rcu_head rcu;
......
...@@ -132,33 +132,17 @@ static void vlan_rcu_free(struct rcu_head *rcu) ...@@ -132,33 +132,17 @@ static void vlan_rcu_free(struct rcu_head *rcu)
vlan_group_free(container_of(rcu, struct vlan_group, rcu)); vlan_group_free(container_of(rcu, struct vlan_group, rcu));
} }
void unregister_vlan_dev(struct net_device *dev)
/* This returns 0 if everything went fine.
* It will return 1 if the group was killed as a result.
* A negative return indicates failure.
*
* The RTNL lock must be held.
*/
static int unregister_vlan_dev(struct net_device *real_dev,
unsigned short vlan_id)
{ {
struct net_device *dev; struct vlan_dev_info *vlan = VLAN_DEV_INFO(dev);
int real_dev_ifindex = real_dev->ifindex; struct net_device *real_dev = vlan->real_dev;
struct vlan_group *grp; struct vlan_group *grp;
unsigned int i; unsigned short vlan_id = vlan->vlan_id;
int ret;
if (vlan_id >= VLAN_VID_MASK)
return -EINVAL;
ASSERT_RTNL(); ASSERT_RTNL();
grp = __vlan_find_group(real_dev_ifindex);
if (!grp)
return -ENOENT;
dev = vlan_group_get_device(grp, vlan_id); grp = __vlan_find_group(real_dev->ifindex);
if (!dev) BUG_ON(!grp);
return -ENOENT;
vlan_proc_rem_dev(dev); vlan_proc_rem_dev(dev);
...@@ -169,20 +153,12 @@ static int unregister_vlan_dev(struct net_device *real_dev, ...@@ -169,20 +153,12 @@ static int unregister_vlan_dev(struct net_device *real_dev,
real_dev->vlan_rx_kill_vid(real_dev, vlan_id); real_dev->vlan_rx_kill_vid(real_dev, vlan_id);
vlan_group_set_device(grp, vlan_id, NULL); vlan_group_set_device(grp, vlan_id, NULL);
synchronize_net(); grp->nr_vlans--;
/* Caller unregisters (and if necessary, puts) VLAN device, but we synchronize_net();
* get rid of the reference to real_dev here.
*/
dev_put(real_dev);
/* If the group is now empty, kill off the group. */ /* If the group is now empty, kill off the group. */
ret = 0; if (grp->nr_vlans == 0) {
for (i = 0; i < VLAN_VID_MASK; i++)
if (vlan_group_get_device(grp, i))
break;
if (i == VLAN_VID_MASK) {
if (real_dev->features & NETIF_F_HW_VLAN_RX) if (real_dev->features & NETIF_F_HW_VLAN_RX)
real_dev->vlan_rx_register(real_dev, NULL); real_dev->vlan_rx_register(real_dev, NULL);
...@@ -190,23 +166,12 @@ static int unregister_vlan_dev(struct net_device *real_dev, ...@@ -190,23 +166,12 @@ static int unregister_vlan_dev(struct net_device *real_dev,
/* Free the group, after all cpu's are done. */ /* Free the group, after all cpu's are done. */
call_rcu(&grp->rcu, vlan_rcu_free); call_rcu(&grp->rcu, vlan_rcu_free);
ret = 1;
} }
return ret; /* Get rid of the vlan's reference to real_dev */
} dev_put(real_dev);
int unregister_vlan_device(struct net_device *dev)
{
int ret;
ret = unregister_vlan_dev(VLAN_DEV_INFO(dev)->real_dev,
VLAN_DEV_INFO(dev)->vlan_id);
unregister_netdevice(dev); unregister_netdevice(dev);
if (ret == 1)
ret = 0;
return ret;
} }
static void vlan_transfer_operstate(const struct net_device *dev, struct net_device *vlandev) static void vlan_transfer_operstate(const struct net_device *dev, struct net_device *vlandev)
...@@ -291,6 +256,8 @@ int register_vlan_dev(struct net_device *dev) ...@@ -291,6 +256,8 @@ int register_vlan_dev(struct net_device *dev)
* it into our local structure. * it into our local structure.
*/ */
vlan_group_set_device(grp, vlan_id, dev); vlan_group_set_device(grp, vlan_id, dev);
grp->nr_vlans++;
if (ngrp && real_dev->features & NETIF_F_HW_VLAN_RX) if (ngrp && real_dev->features & NETIF_F_HW_VLAN_RX)
real_dev->vlan_rx_register(real_dev, ngrp); real_dev->vlan_rx_register(real_dev, ngrp);
if (real_dev->features & NETIF_F_HW_VLAN_FILTER) if (real_dev->features & NETIF_F_HW_VLAN_FILTER)
...@@ -479,20 +446,16 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event, ...@@ -479,20 +446,16 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
case NETDEV_UNREGISTER: case NETDEV_UNREGISTER:
/* Delete all VLANs for this dev. */ /* Delete all VLANs for this dev. */
for (i = 0; i < VLAN_GROUP_ARRAY_LEN; i++) { for (i = 0; i < VLAN_GROUP_ARRAY_LEN; i++) {
int ret;
vlandev = vlan_group_get_device(grp, i); vlandev = vlan_group_get_device(grp, i);
if (!vlandev) if (!vlandev)
continue; continue;
ret = unregister_vlan_dev(dev, /* unregistration of last vlan destroys group, abort
VLAN_DEV_INFO(vlandev)->vlan_id); * afterwards */
if (grp->nr_vlans == 1)
i = VLAN_GROUP_ARRAY_LEN;
unregister_netdevice(vlandev); unregister_vlan_dev(vlandev);
/* Group was destroyed? */
if (ret == 1)
break;
} }
break; break;
} }
...@@ -598,7 +561,8 @@ static int vlan_ioctl_handler(struct net *net, void __user *arg) ...@@ -598,7 +561,8 @@ static int vlan_ioctl_handler(struct net *net, void __user *arg)
err = -EPERM; err = -EPERM;
if (!capable(CAP_NET_ADMIN)) if (!capable(CAP_NET_ADMIN))
break; break;
err = unregister_vlan_device(dev); unregister_vlan_dev(dev);
err = 0;
break; break;
case GET_VLAN_REALDEV_NAME_CMD: case GET_VLAN_REALDEV_NAME_CMD:
......
...@@ -38,7 +38,7 @@ void vlan_dev_get_vid(const struct net_device *dev, unsigned short *result); ...@@ -38,7 +38,7 @@ void vlan_dev_get_vid(const struct net_device *dev, unsigned short *result);
int vlan_check_real_dev(struct net_device *real_dev, unsigned short vlan_id); int vlan_check_real_dev(struct net_device *real_dev, unsigned short vlan_id);
void vlan_setup(struct net_device *dev); void vlan_setup(struct net_device *dev);
int register_vlan_dev(struct net_device *dev); int register_vlan_dev(struct net_device *dev);
int unregister_vlan_device(struct net_device *dev); void unregister_vlan_dev(struct net_device *dev);
int vlan_netlink_init(void); int vlan_netlink_init(void);
void vlan_netlink_fini(void); void vlan_netlink_fini(void);
......
...@@ -137,11 +137,6 @@ static int vlan_newlink(struct net_device *dev, ...@@ -137,11 +137,6 @@ static int vlan_newlink(struct net_device *dev,
return register_vlan_dev(dev); return register_vlan_dev(dev);
} }
static void vlan_dellink(struct net_device *dev)
{
unregister_vlan_device(dev);
}
static inline size_t vlan_qos_map_size(unsigned int n) static inline size_t vlan_qos_map_size(unsigned int n)
{ {
if (n == 0) if (n == 0)
...@@ -226,7 +221,7 @@ struct rtnl_link_ops vlan_link_ops __read_mostly = { ...@@ -226,7 +221,7 @@ struct rtnl_link_ops vlan_link_ops __read_mostly = {
.validate = vlan_validate, .validate = vlan_validate,
.newlink = vlan_newlink, .newlink = vlan_newlink,
.changelink = vlan_changelink, .changelink = vlan_changelink,
.dellink = vlan_dellink, .dellink = unregister_vlan_dev,
.get_size = vlan_get_size, .get_size = vlan_get_size,
.fill_info = vlan_fill_info, .fill_info = vlan_fill_info,
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment