linux-crypto.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Jordan <daniel.m.jordan@oracle.com>
To: Herbert Xu <herbert@gondor.apana.org.au>,
	Steffen Klassert <steffen.klassert@secunet.com>
Cc: Daniel Jordan <daniel.m.jordan@oracle.com>,
	linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v2] padata: validate cpumask without removed CPU during offline
Date: Fri,  9 Aug 2019 17:06:03 -0400	[thread overview]
Message-ID: <20190809210603.20900-1-daniel.m.jordan@oracle.com> (raw)
In-Reply-To: <20190809192857.26585-2-daniel.m.jordan@oracle.com>

Configuring an instance's parallel mask without any online CPUs...

  echo 2 > /sys/kernel/pcrypt/pencrypt/parallel_cpumask
  echo 0 > /sys/devices/system/cpu/cpu1/online

...crashes like this:

  divide error: 0000 [#1] SMP PTI
  CPU: 4 PID: 281 Comm: modprobe Not tainted 5.2.0-padata-base+ #25
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-<snip>
  RIP: 0010:padata_do_parallel+0xf1/0x270
  ...
  Call Trace:
   pcrypt_do_parallel+0xed/0x1e0 [pcrypt]
   pcrypt_aead_encrypt+0xbf/0xd0 [pcrypt]
   do_mult_aead_op+0x68/0x112 [tcrypt]
   test_mb_aead_speed.constprop.0.cold+0x21a/0x55a [tcrypt]
   do_test+0x2280/0x4ca2 [tcrypt]
   tcrypt_mod_init+0x55/0x1000 [tcrypt]
   ...

The cpumask_weight call in padata_cpu_hash returns 0, causing the
division error, because the mask has no CPUs, which is expected in this
situation.  The problem is __padata_remove_cpu doesn't mark the instance
PADATA_INVALID as expected, which would have made padata_do_parallel
return error before doing the division, because it checks for valid
masks too early.

Fix by moving the checks after the masks have been adjusted for the
offlined CPU.  Only do the second check if the first succeeded to avoid
inadvertently clearing PADATA_INVALID.

Stop the instance unconditionally and start again if the masks are
valid.  Stopping the instance only after an invalid mask is found risks
this div-by-0 crash since a padata_do_parallel call in another task
could happen between cpumask_clear_cpu and padata_validate_cpumask.

Fixes: 33e54450683c ("padata: Handle empty padata cpumasks")
Signed-off-by: Daniel Jordan <daniel.m.jordan@oracle.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---

v2: Don't leave the instance stopped if the masks are valid.

 kernel/padata.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/padata.c b/kernel/padata.c
index d056276a96ce..01460ea1d160 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -702,10 +702,7 @@ static int __padata_remove_cpu(struct padata_instance *pinst, int cpu)
 	struct parallel_data *pd = NULL;
 
 	if (cpumask_test_cpu(cpu, cpu_online_mask)) {
-
-		if (!padata_validate_cpumask(pinst, pinst->cpumask.pcpu) ||
-		    !padata_validate_cpumask(pinst, pinst->cpumask.cbcpu))
-			__padata_stop(pinst);
+		__padata_stop(pinst);
 
 		pd = padata_alloc_pd(pinst, pinst->cpumask.pcpu,
 				     pinst->cpumask.cbcpu);
@@ -716,6 +713,9 @@ static int __padata_remove_cpu(struct padata_instance *pinst, int cpu)
 
 		cpumask_clear_cpu(cpu, pd->cpumask.cbcpu);
 		cpumask_clear_cpu(cpu, pd->cpumask.pcpu);
+		if (padata_validate_cpumask(pinst, pd->cpumask.pcpu) &&
+		    padata_validate_cpumask(pinst, pd->cpumask.cbcpu))
+			__padata_start(pinst);
 	}
 
 	return 0;
-- 
2.22.0


  reply	other threads:[~2019-08-09 21:08 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-09 19:28 [PATCH 1/2] padata: always acquire cpu_hotplug_lock before pinst->lock Daniel Jordan
2019-08-09 19:28 ` [PATCH 2/2] padata: validate cpumask without removed CPU during offline Daniel Jordan
2019-08-09 21:06   ` Daniel Jordan [this message]
2019-08-12 21:02     ` [PATCH 3/2] padata: initialize usable masks to reflect offlined CPU Daniel Jordan
2019-08-22  3:51       ` Herbert Xu
2019-08-22 22:11         ` Daniel Jordan
2019-08-22  3:50     ` [PATCH v2] padata: validate cpumask without removed CPU during offline Herbert Xu
2019-08-22 22:10       ` Daniel Jordan
2019-08-22 22:53         ` Daniel Jordan
2019-08-15  5:15 ` [PATCH 1/2] padata: always acquire cpu_hotplug_lock before pinst->lock Herbert Xu
2019-08-21  4:14   ` Daniel Jordan
2019-08-21  6:43     ` Herbert Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190809210603.20900-1-daniel.m.jordan@oracle.com \
    --to=daniel.m.jordan@oracle.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=steffen.klassert@secunet.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).