[Devel] [PATCH RHEL7 COMMIT] neigh: fix possible DoS due to net iface start/stop loop

Konstantin Khorenko khorenko at virtuozzo.com
Mon Aug 15 15:24:10 MSK 2022


The commit is pushed to "branch-rh7-3.10.0-1160.66.1.vz7.188.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-1160.66.1.vz7.188.9
------>
commit 3526fc5856a2a5bb5840153e19872381e813e112
Author: Denis V. Lunev <den at openvz.org>
Date:   Thu Aug 11 14:48:26 2022 +0300

    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.
    
    https://jira.sw.ru/browse/PSBM-140896
    
    Investigated-by: Alexander Mikhalitsyn <alexander.mikhalitsyn at virtuozzo.com>
    Signed-off-by: Denis V. Lunev <den at openvz.org>
    Cc: Alexey Kuznetsov <kuznet at ms2.inr.ac.ru>
    Cc: Alexander Mikhalitsyn <alexander.mikhalitsyn at virtuozzo.com>
    Cc: Konstantin Khorenko <khorenko at virtuozzo.com>
    
    Signed-off-by: Alexander Mikhalitsyn <alexander.mikhalitsyn at virtuozzo.com>
---
 net/core/neighbour.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index f1f242bb61c8a..01eb711e4bdd2 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -223,14 +223,23 @@ static int neigh_del_timer(struct neighbour *n)
 	return 0;
 }
 
-static void pneigh_queue_purge(struct sk_buff_head *list)
+static void pneigh_queue_purge(struct sk_buff_head *list, struct net *net)
 {
+	unsigned long flags;
 	struct sk_buff *skb;
 
-	while ((skb = skb_dequeue(list)) != NULL) {
-		dev_put(skb->dev);
-		kfree_skb(skb);
+	spin_lock_irqsave(&list->lock, flags);
+	skb = skb_peek(list);
+	while (skb != NULL) {
+		struct sk_buff *skb_next = skb_peek_next(skb, list);
+		if (net == NULL || net_eq(dev_net(skb->dev), net)) {
+			__skb_unlink(skb, list);
+			dev_put(skb->dev);
+			kfree_skb(skb);
+		}
+		skb = skb_next;
 	}
+	spin_unlock_irqrestore(&list->lock, flags);
 }
 
 static void neigh_flush_dev(struct neigh_table *tbl, struct net_device *dev)
@@ -297,9 +306,9 @@ int neigh_ifdown(struct neigh_table *tbl, struct net_device *dev)
 	neigh_flush_dev(tbl, dev);
 	pneigh_ifdown(tbl, dev);
 	write_unlock_bh(&tbl->lock);
-
-	del_timer_sync(&tbl->proxy_timer);
-	pneigh_queue_purge(&tbl->proxy_queue);
+	pneigh_queue_purge(&tbl->proxy_queue, dev_net(dev));
+	if (skb_queue_empty(&tbl->proxy_queue))
+		del_timer_sync(&tbl->proxy_timer);
 	return 0;
 }
 EXPORT_SYMBOL(neigh_ifdown);
@@ -1672,7 +1681,7 @@ int neigh_table_clear(int index, struct neigh_table *tbl)
 	/* It is not clean... Fix it to unload IPv6 module safely */
 	cancel_delayed_work_sync(&tbl->gc_work);
 	del_timer_sync(&tbl->proxy_timer);
-	pneigh_queue_purge(&tbl->proxy_queue);
+	pneigh_queue_purge(&tbl->proxy_queue, NULL);
 	neigh_ifdown(tbl, NULL);
 	if (atomic_read(&tbl->entries))
 		pr_crit("neighbour leakage\n");


More information about the Devel mailing list