Re: [PATCH net 2/3] ipv6: mcast: Fix potential UAF in MLD delayed work

On Sat, Jul 04, 2026 at 07:43:45PM +0000, Eric Dumazet wrote:

quoted

A race condition exists between device teardown and incoming MLD query
processing, leading to a Use-After-Free in the MLD delayed work.

During device destruction, the primary reference to inet6_dev is dropped,
which can drop its refcount to 0. The actual freeing of inet6_dev memory
is deferred via RCU.

Concurrently, the packet receive path runs under RCU read lock and obtains
the inet6_dev pointer. Because the memory is RCU-protected, CPU-0 can
safely dereference inet6_dev even if its refcount has hit 0.

However, if CPU-0 calls igmp6_event_query() and schedules delayed work, it
attempts to acquire a reference using in6_dev_hold(). This increments the
refcount from 0 to 1, triggering a "refcount_t: addition on 0" warning.
Since the inet6_dev memory is still scheduled to be freed after the RCU
grace period, the device is freed while the work is still scheduled.
When the work runs, it accesses the freed memory, causing a kernel panic.

Fix this by using refcount_inc_not_zero() (via a new helper
in6_dev_hold_safe()) to prevent acquiring a reference if the device is
already being destroyed. If the refcount is 0, we do not schedule the work.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Eric Dumazet <edumazet@google.com>

Eric, thanks for taking care of this!

quoted

---
 include/net/addrconf.h |  5 +++++
 net/ipv6/mcast.c       | 38 ++++++++++++++++++++++++++++----------
 2 files changed, 33 insertions(+), 10 deletions(-)

[...]

quoted

@@ -1395,6 +1401,7 @@ static void mld_process_v2(struct inet6_dev *idev, struct mld2_query *mld,
 void igmp6_event_query(struct sk_buff *skb)
 {
      struct inet6_dev *idev = __in6_dev_get(skb->dev);
+     bool put = false;

      if (!idev || idev->dead)
              goto out;

@@ -1402,11 +1409,16 @@ void igmp6_event_query(struct sk_buff *skb)
      spin_lock_bh(&idev->mc_query_lock);
      if (skb_queue_len(&idev->mc_query_queue) < MLD_MAX_SKBS) {
              __skb_queue_tail(&idev->mc_query_queue, skb);

Shouldn't we only enqueue the skb if we managed to take a reference?
Something like [1] (on top of this patch).

If we failed to take a reference, then ipv6_mc_destroy_dev() already ran
and the queue will not be purged, thereby leaking this skb.

quoted

-             if (!mod_delayed_work(mld_wq, &idev->mc_query_work, 0))
-                     in6_dev_hold(idev);
+             if (in6_dev_hold_safe(idev)) {
+                     if (mod_delayed_work(mld_wq, &idev->mc_query_work, 0))
+                             put = true;
+             }
              skb = NULL;
      }
      spin_unlock_bh(&idev->mc_query_lock);
+
+     if (put)
+             in6_dev_put(idev);
 out:
      kfree_skb(skb);
 }

@@ -1570,6 +1582,7 @@ static void mld_query_work(struct work_struct *work)
 void igmp6_event_report(struct sk_buff *skb)
 {
      struct inet6_dev *idev = __in6_dev_get(skb->dev);
+     bool put = false;

      if (!idev || idev->dead)
              goto out;

@@ -1577,11 +1590,16 @@ void igmp6_event_report(struct sk_buff *skb)
      spin_lock_bh(&idev->mc_report_lock);
      if (skb_queue_len(&idev->mc_report_queue) < MLD_MAX_SKBS) {
              __skb_queue_tail(&idev->mc_report_queue, skb);

Same here

quoted

-             if (!mod_delayed_work(mld_wq, &idev->mc_report_work, 0))
-                     in6_dev_hold(idev);
+             if (in6_dev_hold_safe(idev)) {
+                     if (mod_delayed_work(mld_wq, &idev->mc_report_work, 0))
+                             put = true;
+             }
              skb = NULL;
      }
      spin_unlock_bh(&idev->mc_report_lock);
+
+     if (put)
+             in6_dev_put(idev);
 out:
      kfree_skb(skb);
 }

[1]

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 12d22de6f496..aaba4c2aae23 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c

@@ -1408,12 +1408,11 @@ void igmp6_event_query(struct sk_buff *skb)
                goto out;

        spin_lock_bh(&idev->mc_query_lock);
-       if (skb_queue_len(&idev->mc_query_queue) < MLD_MAX_SKBS) {
+       if (skb_queue_len(&idev->mc_query_queue) < MLD_MAX_SKBS &&
+           in6_dev_hold_safe(idev)) {
                __skb_queue_tail(&idev->mc_query_queue, skb);
-               if (in6_dev_hold_safe(idev)) {
-                       if (mod_delayed_work(mld_wq, &idev->mc_query_work, 0))
-                               put = true;
-               }
+               if (mod_delayed_work(mld_wq, &idev->mc_query_work, 0))
+                       put = true;
                skb = NULL;
        }
        spin_unlock_bh(&idev->mc_query_lock);

@@ -1589,12 +1588,11 @@ void igmp6_event_report(struct sk_buff *skb)
                goto out;

        spin_lock_bh(&idev->mc_report_lock);
-       if (skb_queue_len(&idev->mc_report_queue) < MLD_MAX_SKBS) {
+       if (skb_queue_len(&idev->mc_report_queue) < MLD_MAX_SKBS &&
+           in6_dev_hold_safe(idev)) {
                __skb_queue_tail(&idev->mc_report_queue, skb);
-               if (in6_dev_hold_safe(idev)) {
-                       if (mod_delayed_work(mld_wq, &idev->mc_report_work, 0))
-                               put = true;
-               }
+               if (mod_delayed_work(mld_wq, &idev->mc_report_work, 0))
+                       put = true;
                skb = NULL;
        }
        spin_unlock_bh(&idev->mc_report_lock);