Received-SPF: pass (google.com: domain of linux-crypto+bounces-2789-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Wed, 20 Mar 2024 16:51:59 -0700
From: Jerry Snitselaar <jsnitsel@redhat.com>
To: Tom Zanussi <tom.zanussi@linux.intel.com>, 
	Vladis Dronov <vdronov@redhat.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>, 
	"David S. Miller" <davem@davemloft.net>, linux-crypto@vger.kernel.org
Subject: Re: Divide by zero in iaa_crypto during boot of a kdump kernel
Message-ID: <wiux57n6yvk5mkysy3fmsmehzepghkphjwexco6gluhhu2vver@c5lj735s3sd5>
References: <hyf3fggjvnjjdbkk4hvocmlfhbz2wapxjhmppurthqavgvk23m@j47q5vlzb2om>
 <39f73bd559aa96051b4d5c8e42d0ce942194b64f.camel@linux.intel.com>
 <4e917f9a299c46c82e1eb206c26e349fb7d810e7.camel@linux.intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4e917f9a299c46c82e1eb206c26e349fb7d810e7.camel@linux.intel.com>

On Wed, Mar 20, 2024 at 01:13:48PM -0500, Tom Zanussi wrote:
> Hi Jerry,
> 
> On Tue, 2024-03-19 at 17:19 -0500, Tom Zanussi wrote:
> > Hi Jerry,
> > 
> > On Tue, 2024-03-19 at 13:51 -0700, Jerry Snitselaar wrote:
> > > Hi Tom,
> > > 
> > > While looking at a different issue on a GNR system I noticed that
> > > during the boot of the kdump kernel it crashes when probing
> > > iaa_crypto
> > > due to a divide by zero in rebalance_wq_table. The problem is that
> > > the
> > > kdump kernel comes up with a single cpu, and if there are multiple
> > > iaa
> > > devices cpus_per_iaa is going to be calculated to be 0, and then
> > > the
> > > 'if ((cpu % cpus_per_iaa) == 0)' in rebalance_wq_table results in a
> > > divide by zero. I reproduced it with the 6.8 eln kernel, and so far
> > > have reproduced it on GNR, EMR, and SRF systems. I'm assuming the
> > > same
> > > will be the case on and SPR system with IAA devices enabled if I
> > > can
> > > find one.
> > > 
> > 
> > Good catch, I've never tested that before. Thanks for reporting it.
> > 
> > > Should save_iaa_wq return an error if the number of iaa devices is
> > > greater
> > > than the number of cpus?
> > > 
> > 
> > No, you should still be able to use the driver with just one cpu,
> > maybe
> > it just always maps to the same device. I'll take a look and come up
> > with a fix.
> > 
> > Tom
> 
> The patch below fixes it for me. It gets rid of the crash and I was
> able to run some basic tests successfully.
> 
> Tom 
> 

It avoids the crash for me.

Regards,
Jerry

> 
> From 37dc97831c9e12c103115cb5fc9866b42cad7bc5 Mon Sep 17 00:00:00 2001
> From: Tom Zanussi <tom.zanussi@linux.intel.com>
> Date: Wed, 20 Mar 2024 05:37:11 -0700
> Subject: [PATCH] crypto: iaa - Fix nr_cpus < nr_iaa case
> 
> If nr_cpus < nr_iaa, the calculated cpus_per_iaa will be 0, which
> causes a divide-by-0 in rebalance_wq_table().
> 
> Make sure cpus_per_iaa is 1 in that case, and also in the nr_iaa == 0
> case, even though cpus_per_iaa is never used if nr_iaa == 0, for
> paranoia.
> 
> Reported-by: Jerry Snitselaar <jsnitsel@redhat.com>
> Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
> ---
>  drivers/crypto/intel/iaa/iaa_crypto_main.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/crypto/intel/iaa/iaa_crypto_main.c b/drivers/crypto/intel/iaa/iaa_crypto_main.c
> index 1cd304de5388..b2191ade9011 100644
> --- a/drivers/crypto/intel/iaa/iaa_crypto_main.c
> +++ b/drivers/crypto/intel/iaa/iaa_crypto_main.c
> @@ -806,6 +806,8 @@ static int save_iaa_wq(struct idxd_wq *wq)
>  		return -EINVAL;
>  
>  	cpus_per_iaa = (nr_nodes * nr_cpus_per_node) / nr_iaa;
> +	if (!cpus_per_iaa)
> +		cpus_per_iaa = 1;
>  out:
>  	return 0;
>  }
> @@ -821,10 +823,12 @@ static void remove_iaa_wq(struct idxd_wq *wq)
>  		}
>  	}
>  
> -	if (nr_iaa)
> +	if (nr_iaa) {
>  		cpus_per_iaa = (nr_nodes * nr_cpus_per_node) / nr_iaa;
> -	else
> -		cpus_per_iaa = 0;
> +		if (!cpus_per_iaa)
> +			cpus_per_iaa = 1;
> +	} else
> +		cpus_per_iaa = 1;
>  }
>  
>  static int wq_table_add_wqs(int iaa, int cpu)
> -- 
> 2.34.1
> 
>