linux-kernel - Re: [PATCH] ath10k: allocate dummy net

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240322082336.49f110cc@kernel.org>
Date: Fri, 22 Mar 2024 08:23:36 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Breno Leitao <leitao@...ian.org>
Cc: Jeff Johnson <quic_jjohnson@...cinc.com>, Kalle Valo <kvalo@...nel.org>,
 Jeff Johnson <jjohnson@...nel.org>, keescook@...omium.org, "open
 list:NETWORKING DRIVERS (WIRELESS)" <linux-wireless@...r.kernel.org>, "open
 list:QUALCOMM ATHEROS ATH10K WIRELESS DRIVER" <ath10k@...ts.infradead.org>,
 open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ath10k: allocate dummy net_device dynamically

On Fri, 22 Mar 2024 07:58:02 -0700 Breno Leitao wrote:
> > Looks like init_dummy_netdev wipes the netdev structure clean, so I
> > don't think we can use it directly as the setup function, Breno :(  
> 
> Before my patch,  init_dummy_netdev was being also used. The patch was
> basically replacing the init_dummy_netdev by alloc_netdev() with will
> call "setup(dev);" later. 
> 
> -               init_dummy_netdev(&irq_grp->napi_ndev);
> +               irq_grp->napi_ndev = alloc_netdev(0, "dummy", NET_NAME_UNKNOWN,
> +                                                 init_dummy_netdev);
> 
> I am wondering if alloc_netdev() is messing with something instead of
> init_dummy_netdev().

alloc_netdev() allocates some memory and initializes lists which
free_netdev() wants to free, basically. But init_dummy_netdev() does:

	/* Clear everything. Note we don't initialize spinlocks
	 * are they aren't supposed to be taken by any of the
	 * NAPI code and this dummy netdev is supposed to be
	 * only ever used for NAPI polls
	 */
	memset(dev, 0, sizeof(struct net_device));

so all those pointers and init alloc_netdev() did before calling setup
will get wiped.

> Also, Kalle's crash is during rmmod, and not during initialization.
> getting NULL after free_netdev() is called.
> 
> > Maybe we should add a new helper to "alloc dummy netdev" which can
> > call alloc_netdev() with right arguments and do necessary init?  
> 
> What are the right arguments in this case?

I'm not sure we have a noop setup() callback today. If you define a
wrapper to allocate a dummy netdev you can define a new empty function
next to it and pass that as init? Hope I got the question right.