mirror of https://lore.kernel.org/lkml/
 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 5/5] padata: avoid potential UAFs to the padata_shell from padata_reorder()
Date: Wed, 19 Oct 2022 10:37:08 +0200	[thread overview]
Message-ID: <20221019083708.27138-6-nstange@suse.de> (raw)
In-Reply-To: <20221019083708.27138-1-nstange@suse.de>

Even though the parallel_data "pd" instance passed to padata_reorder() is
guaranteed to exist as per the reference held by its callers, the same is
not true for the associated padata_shell, pd->ps. More specifically, once
the last padata_priv request has been completed, either at entry from
padata_reorder() or concurrently to it, the padata API users are well
within their right to free the padata_shell instance.

Note that this is a purely theoretical issue, it has not been actually
observed. Yet it's worth fixing for the sake of robustness.

Exploit the fact that as long as there are any not yet completed
padata_priv's around on any of the percpu reorder queues, pd->ps is
guaranteed to exist. Make padata_reorder() to load from pd->ps only when
it's known that there is at least one in-flight padata_priv object to
reorder. Note that this involves moving pd->ps accesses to under the
reorder->lock as appropriate, so that the found padata_priv object won't
get dequeued and completed concurrently from a different context.

Fixes: bbefa1dd6a6d ("crypto: pcrypt - Avoid deadlock by using per-instance
                      padata queues")
Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 kernel/padata.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/kernel/padata.c b/kernel/padata.c
index e9eab3e94cfc..fa4818b81eca 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -286,7 +286,6 @@ static struct padata_priv *padata_dequeue_next(struct parallel_data *pd)
 
 static bool padata_reorder(struct parallel_data *pd)
 {
-	struct padata_instance *pinst = pd->ps->pinst;
 	int cb_cpu;
 	struct padata_priv *padata;
 	struct padata_serial_queue *squeue;
@@ -323,7 +322,11 @@ static bool padata_reorder(struct parallel_data *pd)
 		list_add_tail(&padata->list, &squeue->serial.list);
 		spin_unlock(&squeue->serial.lock);
 
-		queue_work_on(cb_cpu, pinst->serial_wq, &squeue->work);
+		/*
+		 * Note: as long as there are requests in-flight,
+		 * pd->ps is guaranteed to exist.
+		 */
+		queue_work_on(cb_cpu, pd->ps->pinst->serial_wq, &squeue->work);
 	}
 
 	spin_unlock_bh(&pd->lock);
@@ -340,14 +343,23 @@ static bool padata_reorder(struct parallel_data *pd)
 
 	reorder = per_cpu_ptr(pd->reorder_list, pd->cpu);
 	if (!list_empty(&reorder->list)) {
+		bool reenqueued;
+
 		spin_lock(&reorder->lock);
 		if (!__padata_find_next(pd, reorder)) {
 			spin_unlock(&reorder->lock);
 			return false;
 		}
+
+		/*
+		 * Note: as long as there are requests in-flight,
+		 * pd->ps is guaranteed to exist.
+		 */
+		reenqueued = queue_work(pd->ps->pinst->serial_wq,
+					&pd->reorder_work);
 		spin_unlock(&reorder->lock);
 
-		return queue_work(pinst->serial_wq, &pd->reorder_work);
+		return reenqueued;
 	}
 
 	return false;
-- 
2.37.3


  parent reply	other threads:[~2022-10-19  8:38 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 ` [PATCH 2/5] padata: make padata_free_shell() to respect pd's ->refcnt Nicolai Stange
2022-10-28 14:35   ` 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 ` Nicolai Stange [this message]
2022-10-28 16:14   ` [PATCH 5/5] padata: avoid potential UAFs to the padata_shell from padata_reorder() 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-6-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

all inboxes | Powered by JetHome®