Commit 3fc2de2f authored by stephen hemminger's avatar stephen hemminger Committed by David S. Miller

vxlan: fix igmp races

There are two race conditions in existing code for doing IGMP
management in workqueue in vxlan. First, the vxlan_group_used
function checks the list of vxlan's without any protection, and
it is possible for open followed by close to occur before the
igmp work queue runs.

To solve these move the check into vxlan_open/stop so it is
protected by RTNL. And split into two work structures so that
there is no racy reference to underlying device state.
Signed-off-by: default avatarStephen Hemminger <stephen@networkplumber.org>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 372675a4
...@@ -136,7 +136,8 @@ struct vxlan_dev { ...@@ -136,7 +136,8 @@ struct vxlan_dev {
u32 flags; /* VXLAN_F_* below */ u32 flags; /* VXLAN_F_* below */
struct work_struct sock_work; struct work_struct sock_work;
struct work_struct igmp_work; struct work_struct igmp_join;
struct work_struct igmp_leave;
unsigned long age_interval; unsigned long age_interval;
struct timer_list age_timer; struct timer_list age_timer;
...@@ -736,7 +737,6 @@ static bool vxlan_snoop(struct net_device *dev, ...@@ -736,7 +737,6 @@ static bool vxlan_snoop(struct net_device *dev,
return false; return false;
} }
/* See if multicast group is already in use by other ID */ /* See if multicast group is already in use by other ID */
static bool vxlan_group_used(struct vxlan_net *vn, __be32 remote_ip) static bool vxlan_group_used(struct vxlan_net *vn, __be32 remote_ip)
{ {
...@@ -770,12 +770,13 @@ static void vxlan_sock_release(struct vxlan_net *vn, struct vxlan_sock *vs) ...@@ -770,12 +770,13 @@ static void vxlan_sock_release(struct vxlan_net *vn, struct vxlan_sock *vs)
queue_work(vxlan_wq, &vs->del_work); queue_work(vxlan_wq, &vs->del_work);
} }
/* Callback to update multicast group membership. /* Callback to update multicast group membership when first VNI on
* Scheduled when vxlan goes up/down. * multicast asddress is brought up
* Done as workqueue because ip_mc_join_group acquires RTNL.
*/ */
static void vxlan_igmp_work(struct work_struct *work) static void vxlan_igmp_join(struct work_struct *work)
{ {
struct vxlan_dev *vxlan = container_of(work, struct vxlan_dev, igmp_work); struct vxlan_dev *vxlan = container_of(work, struct vxlan_dev, igmp_join);
struct vxlan_net *vn = net_generic(dev_net(vxlan->dev), vxlan_net_id); struct vxlan_net *vn = net_generic(dev_net(vxlan->dev), vxlan_net_id);
struct vxlan_sock *vs = vxlan->vn_sock; struct vxlan_sock *vs = vxlan->vn_sock;
struct sock *sk = vs->sock->sk; struct sock *sk = vs->sock->sk;
...@@ -785,9 +786,26 @@ static void vxlan_igmp_work(struct work_struct *work) ...@@ -785,9 +786,26 @@ static void vxlan_igmp_work(struct work_struct *work)
}; };
lock_sock(sk); lock_sock(sk);
if (vxlan_group_used(vn, vxlan->default_dst.remote_ip))
ip_mc_join_group(sk, &mreq); ip_mc_join_group(sk, &mreq);
else release_sock(sk);
vxlan_sock_release(vn, vs);
dev_put(vxlan->dev);
}
/* Inverse of vxlan_igmp_join when last VNI is brought down */
static void vxlan_igmp_leave(struct work_struct *work)
{
struct vxlan_dev *vxlan = container_of(work, struct vxlan_dev, igmp_leave);
struct vxlan_net *vn = net_generic(dev_net(vxlan->dev), vxlan_net_id);
struct vxlan_sock *vs = vxlan->vn_sock;
struct sock *sk = vs->sock->sk;
struct ip_mreqn mreq = {
.imr_multiaddr.s_addr = vxlan->default_dst.remote_ip,
.imr_ifindex = vxlan->default_dst.remote_ifindex,
};
lock_sock(sk);
ip_mc_leave_group(sk, &mreq); ip_mc_leave_group(sk, &mreq);
release_sock(sk); release_sock(sk);
...@@ -1359,6 +1377,7 @@ static void vxlan_uninit(struct net_device *dev) ...@@ -1359,6 +1377,7 @@ static void vxlan_uninit(struct net_device *dev)
/* Start ageing timer and join group when device is brought up */ /* Start ageing timer and join group when device is brought up */
static int vxlan_open(struct net_device *dev) static int vxlan_open(struct net_device *dev)
{ {
struct vxlan_net *vn = net_generic(dev_net(dev), vxlan_net_id);
struct vxlan_dev *vxlan = netdev_priv(dev); struct vxlan_dev *vxlan = netdev_priv(dev);
struct vxlan_sock *vs = vxlan->vn_sock; struct vxlan_sock *vs = vxlan->vn_sock;
...@@ -1366,10 +1385,11 @@ static int vxlan_open(struct net_device *dev) ...@@ -1366,10 +1385,11 @@ static int vxlan_open(struct net_device *dev)
if (!vs) if (!vs)
return -ENOTCONN; return -ENOTCONN;
if (IN_MULTICAST(ntohl(vxlan->default_dst.remote_ip))) { if (IN_MULTICAST(ntohl(vxlan->default_dst.remote_ip)) &&
! vxlan_group_used(vn, vxlan->default_dst.remote_ip)) {
vxlan_sock_hold(vs); vxlan_sock_hold(vs);
dev_hold(dev); dev_hold(dev);
queue_work(vxlan_wq, &vxlan->igmp_work); queue_work(vxlan_wq, &vxlan->igmp_join);
} }
if (vxlan->age_interval) if (vxlan->age_interval)
...@@ -1400,13 +1420,15 @@ static void vxlan_flush(struct vxlan_dev *vxlan) ...@@ -1400,13 +1420,15 @@ static void vxlan_flush(struct vxlan_dev *vxlan)
/* Cleanup timer and forwarding table on shutdown */ /* Cleanup timer and forwarding table on shutdown */
static int vxlan_stop(struct net_device *dev) static int vxlan_stop(struct net_device *dev)
{ {
struct vxlan_net *vn = net_generic(dev_net(dev), vxlan_net_id);
struct vxlan_dev *vxlan = netdev_priv(dev); struct vxlan_dev *vxlan = netdev_priv(dev);
struct vxlan_sock *vs = vxlan->vn_sock; struct vxlan_sock *vs = vxlan->vn_sock;
if (vs && IN_MULTICAST(ntohl(vxlan->default_dst.remote_ip))) { if (vs && IN_MULTICAST(ntohl(vxlan->default_dst.remote_ip)) &&
! vxlan_group_used(vn, vxlan->default_dst.remote_ip)) {
vxlan_sock_hold(vs); vxlan_sock_hold(vs);
dev_hold(dev); dev_hold(dev);
queue_work(vxlan_wq, &vxlan->igmp_work); queue_work(vxlan_wq, &vxlan->igmp_leave);
} }
del_timer_sync(&vxlan->age_timer); del_timer_sync(&vxlan->age_timer);
...@@ -1471,7 +1493,8 @@ static void vxlan_setup(struct net_device *dev) ...@@ -1471,7 +1493,8 @@ static void vxlan_setup(struct net_device *dev)
INIT_LIST_HEAD(&vxlan->next); INIT_LIST_HEAD(&vxlan->next);
spin_lock_init(&vxlan->hash_lock); spin_lock_init(&vxlan->hash_lock);
INIT_WORK(&vxlan->igmp_work, vxlan_igmp_work); INIT_WORK(&vxlan->igmp_join, vxlan_igmp_join);
INIT_WORK(&vxlan->igmp_leave, vxlan_igmp_leave);
INIT_WORK(&vxlan->sock_work, vxlan_sock_work); INIT_WORK(&vxlan->sock_work, vxlan_sock_work);
init_timer_deferrable(&vxlan->age_timer); init_timer_deferrable(&vxlan->age_timer);
......
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