netdev - Re: [PATCH net-next v3 3/4] crypto: caam: Unembed 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 for Android: free password hash cracker in your pocket

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

Message-ID: <ZobKod5Fhf1kvLp1@gmail.com>
Date: Thu, 4 Jul 2024 09:15:29 -0700
From: Breno Leitao <leitao@...ian.org>
To: Jakub Kicinski <kuba@...nel.org>
Cc: horia.geanta@....com, pankaj.gupta@....com, gaurav.jain@....com,
	linux-crypto@...r.kernel.org,
	Herbert Xu <herbert@...dor.apana.org.au>,
	"David S. Miller" <davem@...emloft.net>, horms@...nel.org,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v3 3/4] crypto: caam: Unembed net_dev structure
 from qi

Hello Jakub,

On Wed, Jul 03, 2024 at 07:45:33PM -0700, Jakub Kicinski wrote:
> On Tue,  2 Jul 2024 11:55:53 -0700 Breno Leitao wrote:

> > @@ -751,10 +766,16 @@ int caam_qi_init(struct platform_device *caam_pdev)
> >  		struct caam_qi_pcpu_priv *priv = per_cpu_ptr(&pcpu_qipriv, i);
> >  		struct caam_napi *caam_napi = &priv->caam_napi;
> >  		struct napi_struct *irqtask = &caam_napi->irqtask;
> > -		struct net_device *net_dev = &priv->net_dev;
> > +		struct net_device *net_dev;
> >  
> > +		net_dev = alloc_netdev_dummy(0);
> > +		if (!net_dev) {
> > +			err = -ENOMEM;
> > +			goto fail;
> 
> free_netdev() doesn't take NULL, free_caam_qi_pcpu_netdev()
> will feed it one if we fail here

Sorry, I am not sure I followed you. Let me ask a clarifying questions:

Do you think that free_netdev() will take NULL ?

If that is the case, that *shouldn't* happen, since I have a cpumask
that tracks the percpu netdev that got allocated, and only free those
percpu-net_device that was properly allocated.

Let me simplify the code to make it easy to understand what I had in
mind:

	int caam_qi_init(struct platform_device *caam_pdev) {
		cpumask_clear(&clean_mask);

		net_dev = alloc_netdev_dummy(0);
		if (!net_dev)
			goto fail;

		cpumask_set_cpu(i, &clean_mask);

	fail:
		free_caam_qi_pcpu_netdev(&clean_mask);
	}

	static void free_caam_qi_pcpu_netdev(const cpumask_t *cpus) {
		for_each_cpu(i, cpus) {
			priv = per_cpu_ptr(&pcpu_qipriv, i);
			free_netdev(priv->net_dev);
		}
	}

So, if alloc_netdev_dummy() fails, then the cpu current cpu will not be
set in `clean_mask`, thus, free_caam_qi_pcpu_netdev() will not free it
later.

Anyway, let me know if I am missing something.