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

From: Breno Leitao

Date: Thu Jun 25 2026 - 06:57:02 EST


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