Received-SPF: pass (google.com: domain of linux-crypto+bounces-2786-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <4e917f9a299c46c82e1eb206c26e349fb7d810e7.camel@linux.intel.com>
Subject: Re: Divide by zero in iaa_crypto during boot of a kdump kernel
From: Tom Zanussi <tom.zanussi@linux.intel.com>
To: Jerry Snitselaar <jsnitsel@redhat.com>, Herbert Xu
	 <herbert@gondor.apana.org.au>, "David S. Miller" <davem@davemloft.net>, 
	linux-crypto@vger.kernel.org
Date: Wed, 20 Mar 2024 13:13:48 -0500
In-Reply-To: <39f73bd559aa96051b4d5c8e42d0ce942194b64f.camel@linux.intel.com>
References: 
	<hyf3fggjvnjjdbkk4hvocmlfhbz2wapxjhmppurthqavgvk23m@j47q5vlzb2om>
	 <39f73bd559aa96051b4d5c8e42d0ce942194b64f.camel@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4-0ubuntu2 
Precedence: bulk
MIME-Version: 1.0

Hi Jerry,

On Tue, 2024-03-19 at 17:19 -0500, Tom Zanussi wrote:
> Hi Jerry,
>=20
> On Tue, 2024-03-19 at 13:51 -0700, Jerry Snitselaar wrote:
> > Hi Tom,
> >=20
> > 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) =3D=3D 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.
> >=20
>=20
> Good catch, I've never tested that before. Thanks for reporting it.
>=20
> > Should save_iaa_wq return an error if the number of iaa devices is
> > greater
> > than the number of cpus?
> >=20
>=20
> 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.
>=20
> 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=20


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 =3D=3D 0
case, even though cpus_per_iaa is never used if nr_iaa =3D=3D 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/in=
tel/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;
=20
 	cpus_per_iaa =3D (nr_nodes * nr_cpus_per_node) / nr_iaa;
+	if (!cpus_per_iaa)
+		cpus_per_iaa =3D 1;
 out:
 	return 0;
 }
@@ -821,10 +823,12 @@ static void remove_iaa_wq(struct idxd_wq *wq)
 		}
 	}
=20
-	if (nr_iaa)
+	if (nr_iaa) {
 		cpus_per_iaa =3D (nr_nodes * nr_cpus_per_node) / nr_iaa;
-	else
-		cpus_per_iaa =3D 0;
+		if (!cpus_per_iaa)
+			cpus_per_iaa =3D 1;
+	} else
+		cpus_per_iaa =3D 1;
 }
=20
 static int wq_table_add_wqs(int iaa, int cpu)
--=20
2.34.1