1. Patchew
  2. linux
  3. View series

[PATCH] crypto: caam/jr: fix use-after-free in interrupt handler teardown

Rosen Penev posted 1 patch 2 months ago 
drivers/crypto/caam/jr.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
[PATCH] crypto: caam/jr: fix use-after-free in interrupt handler teardown
Posted by Rosen Penev 2 months ago 
caam_jr_shutdown() currently calls tasklet_kill() without freeing the
IRQ first, creating a window where the interrupt handler can fire and
schedule the tasklet after it has been killed.  Move from
devm_request_irq() to request_irq()/free_irq() so that the IRQ is
explicitly freed in caam_jr_shutdown() before the tasklet is killed.

As part of this change, pass jrp directly as the dev_id cookie to
request_irq() instead of the device pointer.  This lets the interrupt
handler obtain jrp directly from the cookie, eliminating the
dev_get_drvdata() call in the hot path.

Two related fixes:
- Use jrp->dev instead of the now-undefined dev local in the handler's
  error path.
- Set jrpriv->dev before calling caam_jr_init() so that jrp->dev is
  valid before the IRQ is registered and the handler can run.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/crypto/caam/jr.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index bddeaaaca487..b577c6a2e4aa 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -173,6 +173,7 @@ static int caam_jr_shutdown(struct device *dev)
 
 	ret = caam_reset_hw_jr(dev);
 
+	free_irq(jrp->irq, jrp);
 	tasklet_kill(&jrp->irqtask);
 
 	return ret;
@@ -218,8 +219,7 @@ static void caam_jr_remove(struct platform_device *pdev)
 /* Main per-ring interrupt handler */
 static irqreturn_t caam_jr_interrupt(int irq, void *st_dev)
 {
-	struct device *dev = st_dev;
-	struct caam_drv_private_jr *jrp = dev_get_drvdata(dev);
+	struct caam_drv_private_jr *jrp = st_dev;
 	u32 irqstate;
 
 	/*
@@ -236,7 +236,7 @@ static irqreturn_t caam_jr_interrupt(int irq, void *st_dev)
 	 * restart the queue (and fix code).
 	 */
 	if (irqstate & JRINT_JR_ERROR) {
-		dev_err(dev, "job ring error: irqstate: %08x\n", irqstate);
+		dev_err(jrp->dev, "job ring error: irqstate: %08x\n", irqstate);
 		BUG();
 	}
 
@@ -563,8 +563,8 @@ static int caam_jr_init(struct device *dev)
 		     (unsigned long)&jrp->tasklet_params);
 
 	/* Connect job ring interrupt handler. */
-	error = devm_request_irq(dev, jrp->irq, caam_jr_interrupt, IRQF_SHARED,
-				 dev_name(dev), dev);
+	error = request_irq(jrp->irq, caam_jr_interrupt, IRQF_SHARED,
+				 dev_name(dev), jrp);
 	if (error) {
 		dev_err(dev, "can't connect JobR %d interrupt (%d)\n",
 			jrp->ridx, jrp->irq);
@@ -658,12 +658,12 @@ static int caam_jr_probe(struct platform_device *pdev)
 	if (error)
 		return error;
 
+	jrpriv->dev = jrdev;
+
 	/* Now do the platform independent part */
 	error = caam_jr_init(jrdev); /* now turn on hardware */
 	if (error)
 		return error;
-
-	jrpriv->dev = jrdev;
 	spin_lock(&driver_data.jr_alloc_lock);
 	list_add_tail(&jrpriv->list_node, &driver_data.jr_list);
 	spin_unlock(&driver_data.jr_alloc_lock);
-- 
2.55.0
Re: [PATCH] crypto: caam/jr: fix use-after-free in interrupt handler teardown
Posted by Herbert Xu 1 month, 3 weeks ago 
On Thu, Jul 30, 2026 at 04:16:03PM -0700, Rosen Penev wrote:
> caam_jr_shutdown() currently calls tasklet_kill() without freeing the
> IRQ first, creating a window where the interrupt handler can fire and
> schedule the tasklet after it has been killed.  Move from
> devm_request_irq() to request_irq()/free_irq() so that the IRQ is
> explicitly freed in caam_jr_shutdown() before the tasklet is killed.
> 
> As part of this change, pass jrp directly as the dev_id cookie to
> request_irq() instead of the device pointer.  This lets the interrupt
> handler obtain jrp directly from the cookie, eliminating the
> dev_get_drvdata() call in the hot path.
> 
> Two related fixes:
> - Use jrp->dev instead of the now-undefined dev local in the handler's
>   error path.
> - Set jrpriv->dev before calling caam_jr_init() so that jrp->dev is
>   valid before the IRQ is registered and the handler can run.
> 
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/crypto/caam/jr.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)

I think we should fix the general problem of hardware removal
in the middle of a crypto operation first before attempting these
fixes.

The driver needs to be modified so that it can fail gracefully
if the hardware is forcefully unbound while a crypto op is ongoing.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: caam/jr: fix use-after-free in interrupt handler teardown
Posted by Rosen Penev 1 month, 2 weeks ago 
On Mon, Aug 10, 2026 at 1:10 AM Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Thu, Jul 30, 2026 at 04:16:03PM -0700, Rosen Penev wrote:
> > caam_jr_shutdown() currently calls tasklet_kill() without freeing the
> > IRQ first, creating a window where the interrupt handler can fire and
> > schedule the tasklet after it has been killed.  Move from
> > devm_request_irq() to request_irq()/free_irq() so that the IRQ is
> > explicitly freed in caam_jr_shutdown() before the tasklet is killed.
> >
> > As part of this change, pass jrp directly as the dev_id cookie to
> > request_irq() instead of the device pointer.  This lets the interrupt
> > handler obtain jrp directly from the cookie, eliminating the
> > dev_get_drvdata() call in the hot path.
> >
> > Two related fixes:
> > - Use jrp->dev instead of the now-undefined dev local in the handler's
> >   error path.
> > - Set jrpriv->dev before calling caam_jr_init() so that jrp->dev is
> >   valid before the IRQ is registered and the handler can run.
> >
> > Assisted-by: opencode:big-pickle
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >  drivers/crypto/caam/jr.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
>
> I think we should fix the general problem of hardware removal
> in the middle of a crypto operation first before attempting these
> fixes.
>
> The driver needs to be modified so that it can fail gracefully
> if the hardware is forcefully unbound while a crypto op is ongoing.
That's a much bigger change than this one, which I cannot test.
>
> Thanks,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: caam/jr: fix use-after-free in interrupt handler teardown
Posted by Herbert Xu 1 month, 2 weeks ago 
On Wed, Aug 12, 2026 at 11:54:00AM -0700, Rosen Penev wrote:
>
> > The driver needs to be modified so that it can fail gracefully
> > if the hardware is forcefully unbound while a crypto op is ongoing.
> That's a much bigger change than this one, which I cannot test.

The thing is that as Sashiko has pointed out, if we apply fixes
like this without solving the bigger problem, it'll just make that
issue worse.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.