linux-crypto.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nicolai Stange <nstange@suse.de>
To: Steffen Klassert <steffen.klassert@secunet.com>,
	Daniel Jordan <daniel.m.jordan@oracle.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>,
	Martin Doucha <mdoucha@suse.cz>,
	linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org,
	Nicolai Stange <nstange@suse.de>
Subject: [PATCH 2/5] padata: make padata_free_shell() to respect pd's ->refcnt
Date: Wed, 19 Oct 2022 10:37:05 +0200	[thread overview]
Message-ID: <20221019083708.27138-3-nstange@suse.de> (raw)
In-Reply-To: <20221019083708.27138-1-nstange@suse.de>

On a PREEMPT kernel, the following has been observed while running
pcrypt_aead01 from LTP:

  [ ] general protection fault: 0000 [#1] PREEMPT_RT SMP PTI
  <...>
  [ ] Workqueue: pdecrypt_parallel padata_parallel_worker
  [ ] RIP: 0010:padata_reorder+0x19/0x120
  <...>
  [ ] Call Trace:
  [ ]  padata_parallel_worker+0xa3/0xf0
  [ ]  process_one_work+0x1db/0x4a0
  [ ]  worker_thread+0x2d/0x3c0
  [ ]  ? process_one_work+0x4a0/0x4a0
  [ ]  kthread+0x159/0x180
  [ ]  ? kthread_park+0xb0/0xb0
  [ ]  ret_from_fork+0x35/0x40

The pcrypt_aead01 testcase basically runs a NEWALG/DELALG sequence for some
fixed pcrypt instance in a loop, back to back.

The problem is that once the last ->serial() in padata_serial_worker() is
getting invoked, the pcrypt requests from the selftests would signal
completion, and pcrypt_aead01 can move on and subsequently issue a DELALG.
Upon pcrypt instance deregistration, the associated padata_shell would get
destroyed, which in turn would unconditionally free the associated
parallel_data instance.

If padata_serial_worker() now resumes operation after e.g. having
previously been preempted upon the return from the last of those ->serial()
callbacks, its subsequent accesses to pd for managing the ->refcnt will
all be UAFs. In particular, if the memory backing pd has meanwhile been
reused for some new parallel_data allocation, e.g in the course of
processing another subsequent NEWALG request, the padata_serial_worker()
might find the initial ->refcnt of one and free pd from under that NEWALG
or the associated selftests respectively, leading to "secondary" UAFs such
as in the Oops above.

Note that as it currently stands, a padata_shell owns a reference on its
associated parallel_data already. So fix the UAF in padata_serial_worker()
by making padata_free_shell() to not unconditionally free the shell's
associated parallel_data, but to properly drop that reference via
padata_put_pd() instead.

Fixes: 07928d9bfc81 ("padata: Remove broken queue flushing")
Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 kernel/padata.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/padata.c b/kernel/padata.c
index 3bd1e23f089b..0bf8c80dad5a 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -1112,7 +1112,7 @@ void padata_free_shell(struct padata_shell *ps)
 
 	mutex_lock(&ps->pinst->lock);
 	list_del(&ps->list);
-	padata_free_pd(rcu_dereference_protected(ps->pd, 1));
+	padata_put_pd(rcu_dereference_protected(ps->pd, 1));
 	mutex_unlock(&ps->pinst->lock);
 
 	kfree(ps);
-- 
2.37.3


  parent reply	other threads:[~2022-10-19  8:37 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-19  8:37 [PATCH 0/5] padata: fix liftime issues after ->serial() has completed Nicolai Stange
2022-10-19  8:37 ` [PATCH 1/5] padata: introduce internal padata_get/put_pd() helpers Nicolai Stange
2022-10-28 14:23   ` Daniel Jordan
2022-10-19  8:37 ` Nicolai Stange [this message]
2022-10-28 14:35   ` [PATCH 2/5] padata: make padata_free_shell() to respect pd's ->refcnt Daniel Jordan
2022-10-28 16:22     ` Daniel Jordan
2022-11-09 13:02     ` Nicolai Stange
2022-11-10 22:05       ` Daniel Jordan
2022-10-19  8:37 ` [PATCH 3/5] padata: grab parallel_data refcnt for reorder Nicolai Stange
2022-10-28 16:04   ` Daniel Jordan
2022-11-09 13:03     ` Nicolai Stange
2022-11-10 23:16       ` Daniel Jordan
2024-09-11 10:29   ` Herbert Xu
2022-10-19  8:37 ` [PATCH 4/5] padata: split out dequeue operation from padata_find_next() Nicolai Stange
2022-10-19  8:37 ` [PATCH 5/5] padata: avoid potential UAFs to the padata_shell from padata_reorder() Nicolai Stange
2022-10-28 16:14   ` Daniel Jordan
2022-11-09 13:03     ` Nicolai Stange
2022-11-10 23:22       ` Daniel Jordan
2022-10-21 21:35 ` [PATCH 0/5] padata: fix liftime issues after ->serial() has completed Daniel Jordan
2022-10-24  8:47   ` Nicolai Stange

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=20221019083708.27138-3-nstange@suse.de \
    --to=nstange@suse.de \
    --cc=daniel.m.jordan@oracle.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mdoucha@suse.cz \
    --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).