Re: [PATCH net-next 1/4] netconsole: finish enabling the target before releasing RTNL

[Gustavo Luiz Duarte]

On Thu, Oct 1, 2026 at 4:03 PM Breno Leitao <leitao@xxxxxxxxxx> wrote:
>
> On Mon, Sep 28, 2026 at 07:00:03PM +0100, Gustavo Luiz Duarte wrote:
> > netcons_netpoll_setup() publishes nt->np.dev inside __netpoll_setup(),
> > then releases the RTNL and waits for an RCU grace period before
> > returning. Only after that does its caller store STATE_ENABLED:
> >
> > A NETDEV_UNREGISTER event landing in that window tears the target down
> > and we end up storing STATE_ENABLED with a NULL np->dev, leading to a
> > null-ptr-deref in netconsole_write():
> >
> >   BUG: KASAN: null-ptr-deref in netconsole_write+0xb9/0x7f0
> >   Read of size 8 at addr 00000000000000a8 by task pr/netcon_ext0/135
> >    netconsole_write+0xb9/0x7f0
> >    nbcon_emit_next_record+0x501/0x550
> >    nbcon_emit_one+0x10e/0x170
> >    nbcon_kthread_func+0x2ff/0x3b0
> >    kthread+0x199/0x1e0
>
> Oh gosh. Thanks for hte fix.
>
> > Store STATE_ENABLED before dropping the RTNL lock to avoid racing with
> > netconsole_netdev_event().
> >
> > Fixes: 2382b15bcc39 ("netconsole: take care of NETDEV_UNREGISTER event")
>
> This should go to `net` instead of netdev.

I was unsure whether to target net or net-next because these bugs
require root to be triggered.
I can resend it targeting net. I assume the same applies to patch 2/4,
so I will split 1,2 -> net and 3,4 -> net-next.

> >
> > @@ -552,13 +552,16 @@ static int netcons_netpoll_setup(struct netconsole_target *nt)
> >       err = __netpoll_setup(np, ndev);
> >       if (err)
> >               goto put;
> > -     rtnl_unlock();
> >
> >       /* Make sure all NAPI polls which started before dev->npinfo
> >        * was visible have exited before we start calling NAPI poll.
> >        * NAPI skips locking if dev->npinfo is NULL.
> > +      * Hold RTNL until enable is finished so we don't race with
> > +      * netconsole_netdev_event()
> >        */
> > -     synchronize_rcu();
> > +     synchronize_net();
> > +     nt->state = STATE_ENABLED;
> > +     rtnl_unlock();
>
> Why do you need to synchornize-rcu with the RTNL held? Why not enabling
> nt->state, releasing the lock and than synchronizing RCU?

My understanding from the comment above is that we need to synchronize
RCU before allowing netconsole to start calling NAPI poll.
And (nt->state != STATE_ENABLED) is what is currently blocking
netconsole_write() from calling NAPI poll.
So if we enable nt->state before synchronizing RCU, it would regress
the hang fixed in [1].

[1] https://patch.msgid.link/20250726010846.1105875-1-kuba@xxxxxxxxxx