Re: [PATCH net] net: udp_tunnel: fix use-after-free by refcounting udp_tunnel_nic

[Jakub Kicinski <kuba@kernel.org>]

On Thu, 25 Jun 2026 10:47:09 +0800 Jiayuan Chen wrote:
> On 6/25/26 5:57 AM, Jakub Kicinski wrote:
> > On Wed, 24 Jun 2026 17:10:34 +0000 Eric Dumazet wrote:  
> >> Yue Sun reported a use-after-free and debugobjects warning in
> >> udp_tunnel_nic_device_sync_work() during concurrent device operations.
> >>
> >> The state flags of struct udp_tunnel_nic were originally bitfields
> >> sharing a byte, modified concurrently without locking (RCU vs worker).  
> > Can you clarify the path where the bits are modified without locks??
> > My mental model is that this is basically all under rtnl_lock, and
> > Stan added _another_ lock so that drivers can call "sync" / reply
> > without needing rtnl lock, but any changes are still under rtnl_lock.
> >
> > The gap seems to be that we don't check pending under Stan's new lock,
> > since commit 1ead7501094c6 ("udp_tunnel: remove rtnl_lock dependency")
> > did:  
> 
> 
> I think the real problem is that a single work_pending flag can't track 
> the work being queued twice:
> 
> 1. Thread A calls queue_work() -> work_pending = 1.
> 2. The worker gets picked up; workqueue clears the PENDING(internal work 
> queue flag) bit before running the work function.
>     The worker then blocks on rtnl/utn->lock.
> 3. Thread B calls queue_work() again. Since PENDING was already cleared, 
> it enqueues a second
>     instance and sets work_pending = 1.
> 4. A's worker finally gets the lock and does work_pending = 0, runs, 
> returns.
> 5. Now work_pending == 0 but B's instance is still queued. unregister 
> sees 0, frees utn.

Ah, thanks, now I get it. Claude told me the same thing but in 10,000
words and I lost the thread before reading 'til the end... 

In that case:

diff --git a/net/ipv4/udp_tunnel_nic.c b/net/ipv4/udp_tunnel_nic.c
index 9944ed923ddf..3b32a0afa979 100644
--- a/net/ipv4/udp_tunnel_nic.c
+++ b/net/ipv4/udp_tunnel_nic.c
@@ -301,7 +301,7 @@ __udp_tunnel_nic_device_sync(struct net_device *dev, struct udp_tunnel_nic *utn)
 static void
 udp_tunnel_nic_device_sync(struct net_device *dev, struct udp_tunnel_nic *utn)
 {
-       if (!utn->need_sync)
+       if (!utn->need_sync || utn->work_pending)
                return;
 
        queue_work(udp_tunnel_nic_workqueue, &utn->work);