Re: [PATCH net] netpoll: fix a use-after-free on shutdown path

[Breno Leitao <leitao@debian.org>]

On Wed, Jun 24, 2026 at 07:25:13PM -0700, Jakub Kicinski wrote:
> On Mon, 22 Jun 2026 08:01:23 -0700 Breno Leitao wrote:
> > +		 * synchronize_net() does not protect the worker
> > +		 * (queue_process() is not an RCU reader). It fences the
> > +		 * senders -- the real RCU readers -- so they cannot re-arm
> > +		 * tx_work after the np->dev->npinfo was set to NULL.
> > +		 */
> > +		synchronize_net();
> > +		cancel_delayed_work_sync(&npinfo->tx_work);
> 
> Maybe we can avoid the sync_net and the comment by using
> disable_delayed_work_sync() ?

I've been thinking about it, and I think you have a good point.
queue_process() is the only place that take npinfo without RCU
protection.

This is what it happening right now:

CPU0 {
	run tx_work (queue_process())
	npinfo = container_of()...
	while {
A:		deqeue skb from the txq
		try to send
	}
}

CPU 1 {
	call_rcu() -> rcu_cleanup_netpoll_info()
	np->dev->npinfo, NULL
B:	kfree(npinfo);
}

Then, if B happens before A, we have the UAF. That said, if we make sure
that tx_work() is done, then we are OK with rcu_cleanup_netpoll_info

I am not totally sure if the order of pointer zero'ing and disabling
tx work is important, but, it doesn't seem so, any order would be OK
for:

	RCU_INIT_POINTER(np->dev->npinfo, NULL);
	disable_delayed_work_sync(&npinfo->tx_work);

Given that npinfo is not read inside queue_process(), then, order doesn't
matter.

Thanks for the point, I will update.
--breno

---
pw-bot: cr