Skip to content

Commit 66ba215

Browse files
dlunevdavem330
authored andcommitted
neigh: fix possible DoS due to net iface start/stop loop
Normal processing of ARP request (usually this is Ethernet broadcast packet) coming to the host is looking like the following: * the packet comes to arp_process() call and is passed through routing procedure * the request is put into the queue using pneigh_enqueue() if corresponding ARP record is not local (common case for container records on the host) * the request is processed by timer (within 80 jiffies by default) and ARP reply is sent from the same arp_process() using NEIGH_CB(skb)->flags & LOCALLY_ENQUEUED condition (flag is set inside pneigh_enqueue()) And here the problem comes. Linux kernel calls pneigh_queue_purge() which destroys the whole queue of ARP requests on ANY network interface start/stop event through __neigh_ifdown(). This is actually not a problem within the original world as network interface start/stop was accessible to the host 'root' only, which could do more destructive things. But the world is changed and there are Linux containers available. Here container 'root' has an access to this API and could be considered as untrusted user in the hosting (container's) world. Thus there is an attack vector to other containers on node when container's root will endlessly start/stop interfaces. We have observed similar situation on a real production node when docker container was doing such activity and thus other containers on the node become not accessible. The patch proposed doing very simple thing. It drops only packets from the same namespace in the pneigh_queue_purge() where network interface state change is detected. This is enough to prevent the problem for the whole node preserving original semantics of the code. v2: - do del_timer_sync() if queue is empty after pneigh_queue_purge() v3: - rebase to net tree Cc: "David S. Miller" <[email protected]> Cc: Eric Dumazet <[email protected]> Cc: Jakub Kicinski <[email protected]> Cc: Paolo Abeni <[email protected]> Cc: Daniel Borkmann <[email protected]> Cc: David Ahern <[email protected]> Cc: Yajun Deng <[email protected]> Cc: Roopa Prabhu <[email protected]> Cc: Christian Brauner <[email protected]> Cc: [email protected] Cc: [email protected] Cc: Alexey Kuznetsov <[email protected]> Cc: Alexander Mikhalitsyn <[email protected]> Cc: Konstantin Khorenko <[email protected]> Cc: [email protected] Cc: [email protected] Investigated-by: Alexander Mikhalitsyn <[email protected]> Signed-off-by: Denis V. Lunev <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 68a838b commit 66ba215

File tree

1 file changed

+17
-8
lines changed

1 file changed

+17
-8
lines changed

net/core/neighbour.c

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -307,14 +307,23 @@ static int neigh_del_timer(struct neighbour *n)
307307
return 0;
308308
}
309309

310-
static void pneigh_queue_purge(struct sk_buff_head *list)
310+
static void pneigh_queue_purge(struct sk_buff_head *list, struct net *net)
311311
{
312+
unsigned long flags;
312313
struct sk_buff *skb;
313314

314-
while ((skb = skb_dequeue(list)) != NULL) {
315-
dev_put(skb->dev);
316-
kfree_skb(skb);
315+
spin_lock_irqsave(&list->lock, flags);
316+
skb = skb_peek(list);
317+
while (skb != NULL) {
318+
struct sk_buff *skb_next = skb_peek_next(skb, list);
319+
if (net == NULL || net_eq(dev_net(skb->dev), net)) {
320+
__skb_unlink(skb, list);
321+
dev_put(skb->dev);
322+
kfree_skb(skb);
323+
}
324+
skb = skb_next;
317325
}
326+
spin_unlock_irqrestore(&list->lock, flags);
318327
}
319328

320329
static void neigh_flush_dev(struct neigh_table *tbl, struct net_device *dev,
@@ -385,9 +394,9 @@ static int __neigh_ifdown(struct neigh_table *tbl, struct net_device *dev,
385394
write_lock_bh(&tbl->lock);
386395
neigh_flush_dev(tbl, dev, skip_perm);
387396
pneigh_ifdown_and_unlock(tbl, dev);
388-
389-
del_timer_sync(&tbl->proxy_timer);
390-
pneigh_queue_purge(&tbl->proxy_queue);
397+
pneigh_queue_purge(&tbl->proxy_queue, dev_net(dev));
398+
if (skb_queue_empty_lockless(&tbl->proxy_queue))
399+
del_timer_sync(&tbl->proxy_timer);
391400
return 0;
392401
}
393402

@@ -1787,7 +1796,7 @@ int neigh_table_clear(int index, struct neigh_table *tbl)
17871796
cancel_delayed_work_sync(&tbl->managed_work);
17881797
cancel_delayed_work_sync(&tbl->gc_work);
17891798
del_timer_sync(&tbl->proxy_timer);
1790-
pneigh_queue_purge(&tbl->proxy_queue);
1799+
pneigh_queue_purge(&tbl->proxy_queue, NULL);
17911800
neigh_ifdown(tbl, NULL);
17921801
if (atomic_read(&tbl->entries))
17931802
pr_crit("neighbour leakage\n");

0 commit comments

Comments
 (0)