[PATCH net v2] linkwatch: use __dev_put() in callers to prevent UAF

Jiayuan Chen posted 1 patch 1 week ago
There is a newer version of this series
net/core/link_watch.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
[PATCH net v2] linkwatch: use __dev_put() in callers to prevent UAF
Posted by Jiayuan Chen 1 week ago
From: Jiayuan Chen <jiayuan.chen@shopee.com>

After linkwatch_do_dev() calls __dev_put() to release the linkwatch
reference, the device refcount may drop to 1. At this point,
netdev_run_todo() can proceed (since linkwatch_sync_dev() sees an
empty list and returns without blocking), wait for the refcount to
become 1 via netdev_wait_allrefs_any(), and then free the device
via kobject_put().

This creates a use-after-free when __linkwatch_run_queue() tries to
call netdev_unlock_ops() on the already-freed device.

Note that adding netdev_lock_ops()/netdev_unlock_ops() pair in
netdev_run_todo() before kobject_put() would not work, because
netdev_lock_ops() is conditional - it only locks when
netdev_need_ops_lock() returns true. If the device doesn't require
ops_lock, linkwatch won't hold any lock, and netdev_run_todo()
acquiring the lock won't provide synchronization.

Fix this by moving __dev_put() from linkwatch_do_dev() to its
callers. The device reference logically pairs with de-listing the
device, so it's reasonable for the caller that did the de-listing
to release it. This allows placing __dev_put() after all device
accesses are complete, preventing UAF.

The bug can be reproduced by adding mdelay(2000) after
linkwatch_do_dev() in __linkwatch_run_queue(), then running:

  ip tuntap add mode tun name tun_test
  ip link set tun_test up
  ip link set tun_test carrier off
  ip link set tun_test carrier on
  sleep 0.5
  ip tuntap del mode tun name tun_test

KASAN report:

 ==================================================================
 BUG: KASAN: use-after-free in netdev_need_ops_lock include/net/netdev_lock.h:33 [inline]
 BUG: KASAN: use-after-free in netdev_unlock_ops include/net/netdev_lock.h:47 [inline]
 BUG: KASAN: use-after-free in __linkwatch_run_queue+0x865/0x8a0 net/core/link_watch.c:245
 Read of size 8 at addr ffff88804de5c008 by task kworker/u32:10/8123

 CPU: 0 UID: 0 PID: 8123 Comm: kworker/u32:10 Not tainted syzkaller #0 PREEMPT(full)
 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
 Workqueue: events_unbound linkwatch_event
 Call Trace:
  <TASK>
  __dump_stack lib/dump_stack.c:94 [inline]
  dump_stack_lvl+0x100/0x190 lib/dump_stack.c:120
  print_address_description mm/kasan/report.c:378 [inline]
  print_report+0x156/0x4c9 mm/kasan/report.c:482
  kasan_report+0xdf/0x1a0 mm/kasan/report.c:595
  netdev_need_ops_lock include/net/netdev_lock.h:33 [inline]
  netdev_unlock_ops include/net/netdev_lock.h:47 [inline]
  __linkwatch_run_queue+0x865/0x8a0 net/core/link_watch.c:245
  linkwatch_event+0x8f/0xc0 net/core/link_watch.c:304
  process_one_work+0x9c2/0x1840 kernel/workqueue.c:3257
  process_scheduled_works kernel/workqueue.c:3340 [inline]
  worker_thread+0x5da/0xe40 kernel/workqueue.c:3421
  kthread+0x3b3/0x730 kernel/kthread.c:463
  ret_from_fork+0x754/0xaf0 arch/x86/kernel/process.c:158
  ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:246
  </TASK>
 ==================================================================

Fixes: 04efcee6ef8d ("net: hold instance lock during NETDEV_CHANGE")
Reported-by: syzbot+1ec2f6a450f0b54af8c8@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/6824d064.a70a0220.3e9d8.001a.GAE@google.com/T/
Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 net/core/link_watch.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/core/link_watch.c b/net/core/link_watch.c
index 212cde35affa..5196f8ea43f1 100644
--- a/net/core/link_watch.c
+++ b/net/core/link_watch.c
@@ -185,10 +185,6 @@ static void linkwatch_do_dev(struct net_device *dev)
 
 		netif_state_change(dev);
 	}
-	/* Note: our callers are responsible for calling netdev_tracker_free().
-	 * This is the reason we use __dev_put() instead of dev_put().
-	 */
-	__dev_put(dev);
 }
 
 static void __linkwatch_run_queue(int urgent_only)
@@ -243,6 +239,7 @@ static void __linkwatch_run_queue(int urgent_only)
 		netdev_lock_ops(dev);
 		linkwatch_do_dev(dev);
 		netdev_unlock_ops(dev);
+		__dev_put(dev);
 		do_dev--;
 		spin_lock_irq(&lweventlist_lock);
 	}
@@ -278,8 +275,10 @@ void __linkwatch_sync_dev(struct net_device *dev)
 {
 	netdev_ops_assert_locked(dev);
 
-	if (linkwatch_clean_dev(dev))
+	if (linkwatch_clean_dev(dev)) {
 		linkwatch_do_dev(dev);
+		__dev_put(dev);
+	}
 }
 
 void linkwatch_sync_dev(struct net_device *dev)
@@ -288,6 +287,7 @@ void linkwatch_sync_dev(struct net_device *dev)
 		netdev_lock_ops(dev);
 		linkwatch_do_dev(dev);
 		netdev_unlock_ops(dev);
+		__dev_put(dev);
 	}
 }
 
-- 
2.43.0
Re: [PATCH net v2] linkwatch: use __dev_put() in callers to prevent UAF
Posted by Eric Dumazet 1 week ago
On Fri, Jan 30, 2026 at 3:15 PM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>
> From: Jiayuan Chen <jiayuan.chen@shopee.com>
>
> After linkwatch_do_dev() calls __dev_put() to release the linkwatch
> reference, the device refcount may drop to 1. At this point,
> netdev_run_todo() can proceed (since linkwatch_sync_dev() sees an
> empty list and returns without blocking), wait for the refcount to
> become 1 via netdev_wait_allrefs_any(), and then free the device
> via kobject_put().
>
> This creates a use-after-free when __linkwatch_run_queue() tries to
> call netdev_unlock_ops() on the already-freed device.
>
> Note that adding netdev_lock_ops()/netdev_unlock_ops() pair in
> netdev_run_todo() before kobject_put() would not work, because
> netdev_lock_ops() is conditional - it only locks when
> netdev_need_ops_lock() returns true. If the device doesn't require
> ops_lock, linkwatch won't hold any lock, and netdev_run_todo()
> acquiring the lock won't provide synchronization.
>
> Fix this by moving __dev_put() from linkwatch_do_dev() to its
> callers. The device reference logically pairs with de-listing the
> device, so it's reasonable for the caller that did the de-listing
> to release it. This allows placing __dev_put() after all device
> accesses are complete, preventing UAF.
>
> The bug can be reproduced by adding mdelay(2000) after
> linkwatch_do_dev() in __linkwatch_run_queue(), then running:
>
>   ip tuntap add mode tun name tun_test
>   ip link set tun_test up
>   ip link set tun_test carrier off
>   ip link set tun_test carrier on
>   sleep 0.5
>   ip tuntap del mode tun name tun_test
>
> KASAN report:
>
>  ==================================================================
>  BUG: KASAN: use-after-free in netdev_need_ops_lock include/net/netdev_lock.h:33 [inline]
>  BUG: KASAN: use-after-free in netdev_unlock_ops include/net/netdev_lock.h:47 [inline]
>  BUG: KASAN: use-after-free in __linkwatch_run_queue+0x865/0x8a0 net/core/link_watch.c:245
>  Read of size 8 at addr ffff88804de5c008 by task kworker/u32:10/8123
>
>  CPU: 0 UID: 0 PID: 8123 Comm: kworker/u32:10 Not tainted syzkaller #0 PREEMPT(full)
>  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
>  Workqueue: events_unbound linkwatch_event
>  Call Trace:
>   <TASK>
>   __dump_stack lib/dump_stack.c:94 [inline]
>   dump_stack_lvl+0x100/0x190 lib/dump_stack.c:120
>   print_address_description mm/kasan/report.c:378 [inline]
>   print_report+0x156/0x4c9 mm/kasan/report.c:482
>   kasan_report+0xdf/0x1a0 mm/kasan/report.c:595
>   netdev_need_ops_lock include/net/netdev_lock.h:33 [inline]
>   netdev_unlock_ops include/net/netdev_lock.h:47 [inline]
>   __linkwatch_run_queue+0x865/0x8a0 net/core/link_watch.c:245
>   linkwatch_event+0x8f/0xc0 net/core/link_watch.c:304
>   process_one_work+0x9c2/0x1840 kernel/workqueue.c:3257
>   process_scheduled_works kernel/workqueue.c:3340 [inline]
>   worker_thread+0x5da/0xe40 kernel/workqueue.c:3421
>   kthread+0x3b3/0x730 kernel/kthread.c:463
>   ret_from_fork+0x754/0xaf0 arch/x86/kernel/process.c:158
>   ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:246
>   </TASK>
>  ==================================================================
>
> Fixes: 04efcee6ef8d ("net: hold instance lock during NETDEV_CHANGE")
> Reported-by: syzbot+1ec2f6a450f0b54af8c8@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/6824d064.a70a0220.3e9d8.001a.GAE@google.com/T/
> Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
> ---
>  net/core/link_watch.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/net/core/link_watch.c b/net/core/link_watch.c
> index 212cde35affa..5196f8ea43f1 100644
> --- a/net/core/link_watch.c
> +++ b/net/core/link_watch.c
> @@ -185,10 +185,6 @@ static void linkwatch_do_dev(struct net_device *dev)
>
>                 netif_state_change(dev);
>         }
> -       /* Note: our callers are responsible for calling netdev_tracker_free().
> -        * This is the reason we use __dev_put() instead of dev_put().
> -        */
> -       __dev_put(dev);
>  }
>
>  static void __linkwatch_run_queue(int urgent_only)
> @@ -243,6 +239,7 @@ static void __linkwatch_run_queue(int urgent_only)
>                 netdev_lock_ops(dev);
>                 linkwatch_do_dev(dev);
>                 netdev_unlock_ops(dev);

I would add a comment here, referring to netdev_tracker_free(dev,
&dev->linkwatch_dev_tracker)
few lines before this __dev_put()


> +               __dev_put(dev);
>                 do_dev--;
>                 spin_lock_irq(&lweventlist_lock);
>         }
> @@ -278,8 +275,10 @@ void __linkwatch_sync_dev(struct net_device *dev)
>  {
>         netdev_ops_assert_locked(dev);
>
> -       if (linkwatch_clean_dev(dev))
> +       if (linkwatch_clean_dev(dev)) {
>                 linkwatch_do_dev(dev);

A comment is needed as well.

> +               __dev_put(dev);
> +       }
>  }
>
>  void linkwatch_sync_dev(struct net_device *dev)
> @@ -288,6 +287,7 @@ void linkwatch_sync_dev(struct net_device *dev)
>                 netdev_lock_ops(dev);
>                 linkwatch_do_dev(dev);
>                 netdev_unlock_ops(dev);

Same, please add a comment to ease future bug hunting.

> +               __dev_put(dev);
>         }
>  }
>
> --
> 2.43.0
>

Thanks !