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

[Breno Leitao]

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.
>
> @@ -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?

--breno