linux-kernel - [PATCH] crypto: caam/qi - Fix drv

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z_SxYFdyBJTYe_7G@gondor.apana.org.au>
Date: Tue, 8 Apr 2025 13:17:20 +0800
From: Herbert Xu <herbert@...dor.apana.org.au>
To: Sean Anderson <sean.anderson@...ux.dev>
Cc: Horia Geantă <horia.geanta@....com>,
	Pankaj Gupta <pankaj.gupta@....com>,
	Gaurav Jain <gaurav.jain@....com>, linux-crypto@...r.kernel.org,
	"David S. Miller" <davem@...emloft.net>,
	linux-kernel@...r.kernel.org,
	Valentin Ciocoi Radulescu <valentin.ciocoi@....com>
Subject: [PATCH] crypto: caam/qi - Fix drv_ctx refcount bug

On Mon, Apr 07, 2025 at 07:16:38PM -0400, Sean Anderson wrote:
>
> [    2.731344] refcount_t: decrement hit 0; leaking memory.

...

> [    2.731496] caam_rsp_fq_dqrr_cb (include/linux/refcount.h:336 include/linux/refcount.h:351 drivers/crypto/caam/qi.c:593) 
> [    2.731502] qman_p_poll_dqrr (drivers/soc/fsl/qbman/qman.c:1652 drivers/soc/fsl/qbman/qman.c:1759) 
> [    2.731510] caam_qi_poll (drivers/crypto/caam/qi.c:491) 
> [    2.731514] __napi_poll (net/core/dev.c:7328) 
> [    2.731520] net_rx_action (net/core/dev.c:7394 net/core/dev.c:7514) 
> [    2.731524] handle_softirqs (arch/arm64/include/asm/jump_label.h:36 include/trace/events/irq.h:142 kernel/softirq.c:562) 
> [    2.731530] __do_softirq (kernel/softirq.c:596) 
> [    2.731533] ____do_softirq (arch/arm64/kernel/irq.c:82) 
> [    2.731538] call_on_irq_stack (arch/arm64/kernel/entry.S:897) 
> [    2.731542] do_softirq_own_stack (arch/arm64/kernel/irq.c:87) 
> [    2.731547] __irq_exit_rcu (kernel/softirq.c:442 kernel/softirq.c:662) 
> [    2.731550] irq_exit_rcu (kernel/softirq.c:681) 
> [    2.731554] el1_interrupt (arch/arm64/kernel/entry-common.c:565 arch/arm64/kernel/entry-common.c:575) 
> [    2.731561] el1h_64_irq_handler (arch/arm64/kernel/entry-common.c:581) 
> [    2.731567] el1h_64_irq (arch/arm64/kernel/entry.S:596) 
> [    2.731570] qman_enqueue (drivers/soc/fsl/qbman/qman.c:2354) (P)
> [    2.731576] caam_qi_enqueue (drivers/crypto/caam/qi.c:125) 

So caam_qi_enqueue hasn't had a chance to increment the refcount
and the IRQ already came in to decrement it.  Lesson is that you
should always increment your refcount before you give it away.

---8<---
Ensure refcount is raised before request is enqueued since it could
be dequeued before the call returns.

Reported-by: Sean Anderson <sean.anderson@...ux.dev>
Cc: <stable@...r.kernel.org>
Fixes: 11144416a755 ("crypto: caam/qi - optimize frame queue cleanup")
Signed-off-by: Herbert Xu <herbert@...dor.apana.org.au>

diff --git a/drivers/crypto/caam/qi.c b/drivers/crypto/caam/qi.c
index 7701d00bcb3a..b6e7c0b29d4e 100644
--- a/drivers/crypto/caam/qi.c
+++ b/drivers/crypto/caam/qi.c
@@ -122,12 +122,12 @@ int caam_qi_enqueue(struct device *qidev, struct caam_drv_req *req)
 	qm_fd_addr_set64(&fd, addr);
 
 	do {
+		refcount_inc(&req->drv_ctx->refcnt);
 		ret = qman_enqueue(req->drv_ctx->req_fq, &fd);
-		if (likely(!ret)) {
-			refcount_inc(&req->drv_ctx->refcnt);
+		if (likely(!ret))
 			return 0;
-		}
 
+		refcount_dec(&req->drv_ctx->refcnt);
 		if (ret != -EBUSY)
 			break;
 		num_retries++;
-- 
Email: Herbert Xu <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt