[PATCH] crypto: acomp - fix wrong pointer in acomp_reqchain_done()

[PATCH] crypto: acomp - fix wrong pointer in acomp_reqchain_done()

[Giovanni Cabiddu]

acomp_save_req() stores a pointer to req->chain in req->base.data:

    req->base.data = state;  /* state = &req->chain */

When a driver completes asynchronously, acomp_reqchain_done() receives
this pointer as its data argument. However, it incorrectly casts the
data pointer to a struct acomp_req.

Use container_of() to recover the enclosing acomp_req from the chain
member pointer.

Fixes: 64929fe8c0a4 ("crypto: acomp - Remove request chaining")
Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
Reviewed-by: Laurent M Coquerel <laurent.m.coquerel@intel.com>
Reviewed-by: Wojciech Drewek <wojciech.drewek@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 crypto/acompress.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/crypto/acompress.c b/crypto/acompress.c
index 1f9cb04b447f..deb50c078f80 100644
--- a/crypto/acompress.c
+++ b/crypto/acompress.c
@@ -9,6 +9,7 @@
 
 #include <crypto/internal/acompress.h>
 #include <crypto/scatterwalk.h>
+#include <linux/container_of.h>
 #include <linux/cryptouser.h>
 #include <linux/cpumask.h>
 #include <linux/err.h>
@@ -251,7 +252,7 @@ static int acomp_reqchain_finish(struct acomp_req *req, int err)
 
 static void acomp_reqchain_done(void *data, int err)
 {
-	struct acomp_req *req = data;
+	struct acomp_req *req = container_of(data, struct acomp_req, chain);
 	crypto_completion_t compl;
 
 	compl = req->chain.compl;
-- 
2.53.0

Re: [PATCH] crypto: acomp - fix wrong pointer in acomp_reqchain_done()

[Herbert Xu]

On Tue, Mar 24, 2026 at 06:07:09PM +0000, Giovanni Cabiddu wrote:
>
> @@ -251,7 +252,7 @@ static int acomp_reqchain_finish(struct acomp_req *req, int err)
>  
>  static void acomp_reqchain_done(void *data, int err)
>  {
> -	struct acomp_req *req = data;
> +	struct acomp_req *req = container_of(data, struct acomp_req, chain);

How about just passing the request in as data?

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

[PATCH] crypto: acomp - fix wrong pointer stored by acomp_save_req()

[Giovanni Cabiddu]

On Fri, Apr 03, 2026 at 08:44:16AM +0800, Herbert Xu wrote:
> On Tue, Mar 24, 2026 at 06:07:09PM +0000, Giovanni Cabiddu wrote:
> >
> > @@ -251,7 +252,7 @@ static int acomp_reqchain_finish(struct acomp_req *req, int err)
> >  
> >  static void acomp_reqchain_done(void *data, int err)
> >  {
> > -	struct acomp_req *req = data;
> > +	struct acomp_req *req = container_of(data, struct acomp_req, chain);
> 
> How about just passing the request in as data?
Sure! Here is an alternative implementation. Since this is a rewrite, I
decided not to label it as v2.

---8<---
acomp_save_req() stores &req->chain in req->base.data. When
acomp_reqchain_done() is invoked on asynchronous completion, it receives
&req->chain as the data argument but casts it directly to struct
acomp_req. Since data points to the chain member, all subsequent field
accesses are at a wrong offset, resulting in memory corruption.

The issue occurs when an asynchronous hardware implementation, such as
the QAT driver, completes a request that uses the DMA virtual address
interface (e.g. acomp_request_set_src_dma()). This combination causes
crypto_acomp_compress() to enter the acomp_do_req_chain() path, which
sets acomp_reqchain_done() as the completion callback via
acomp_save_req().

With KASAN enabled, this manifests as a general protection fault in
acomp_reqchain_done():

  general protection fault, probably for non-canonical address 0xe000040000000000
  KASAN: probably user-memory-access in range [0x0000400000000000-0x0000400000000007]
  RIP: 0010:acomp_reqchain_done+0x15b/0x4e0
  Call Trace:
   <IRQ>
   qat_comp_alg_callback+0x5d/0xa0 [intel_qat]
   adf_ring_response_handler+0x376/0x8b0 [intel_qat]
   adf_response_handler+0x60/0x170 [intel_qat]
   tasklet_action_common+0x223/0x820
   handle_softirqs+0x1ab/0x640
   </IRQ>

Fix this by storing the request itself in req->base.data instead of
&req->chain, so that acomp_reqchain_done() receives the correct pointer.
Simplify acomp_restore_req() accordingly to access req->chain directly.

Fixes: 64929fe8c0a4 ("crypto: acomp - Remove request chaining")
Cc: stable@vger.kernel.org
Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
---
 crypto/acompress.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/crypto/acompress.c b/crypto/acompress.c
index 1f9cb04b447f..6025c1acce49 100644
--- a/crypto/acompress.c
+++ b/crypto/acompress.c
@@ -169,15 +169,13 @@ static void acomp_save_req(struct acomp_req *req, crypto_completion_t cplt)
 	state->compl = req->base.complete;
 	state->data = req->base.data;
 	req->base.complete = cplt;
-	req->base.data = state;
+	req->base.data = req;
 }
 
 static void acomp_restore_req(struct acomp_req *req)
 {
-	struct acomp_req_chain *state = req->base.data;
-
-	req->base.complete = state->compl;
-	req->base.data = state->data;
+	req->base.complete = req->chain.compl;
+	req->base.data = req->chain.data;
 }
 
 static void acomp_reqchain_virt(struct acomp_req *req)
-- 
2.53.0

Re: [PATCH] crypto: acomp - fix wrong pointer stored by acomp_save_req()

[Herbert Xu]

On Thu, Apr 16, 2026 at 06:07:00PM +0100, Giovanni Cabiddu wrote:
>
> acomp_save_req() stores &req->chain in req->base.data. When
> acomp_reqchain_done() is invoked on asynchronous completion, it receives
> &req->chain as the data argument but casts it directly to struct
> acomp_req. Since data points to the chain member, all subsequent field
> accesses are at a wrong offset, resulting in memory corruption.
> 
> The issue occurs when an asynchronous hardware implementation, such as
> the QAT driver, completes a request that uses the DMA virtual address
> interface (e.g. acomp_request_set_src_dma()). This combination causes
> crypto_acomp_compress() to enter the acomp_do_req_chain() path, which
> sets acomp_reqchain_done() as the completion callback via
> acomp_save_req().
> 
> With KASAN enabled, this manifests as a general protection fault in
> acomp_reqchain_done():
> 
>   general protection fault, probably for non-canonical address 0xe000040000000000
>   KASAN: probably user-memory-access in range [0x0000400000000000-0x0000400000000007]
>   RIP: 0010:acomp_reqchain_done+0x15b/0x4e0
>   Call Trace:
>    <IRQ>
>    qat_comp_alg_callback+0x5d/0xa0 [intel_qat]
>    adf_ring_response_handler+0x376/0x8b0 [intel_qat]
>    adf_response_handler+0x60/0x170 [intel_qat]
>    tasklet_action_common+0x223/0x820
>    handle_softirqs+0x1ab/0x640
>    </IRQ>
> 
> Fix this by storing the request itself in req->base.data instead of
> &req->chain, so that acomp_reqchain_done() receives the correct pointer.
> Simplify acomp_restore_req() accordingly to access req->chain directly.
> 
> Fixes: 64929fe8c0a4 ("crypto: acomp - Remove request chaining")
> Cc: stable@vger.kernel.org
> Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
> ---
>  crypto/acompress.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)

Patch applied.  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