crypto: seqiv: check req->iv only for synchronous completion

[PATCH] crypto: seqiv: check req->iv only for synchronous completion

Posted by Xin Long 1 month, 3 weeks ago

Xiumei reported a UAF crash when intel_qat is used in ipsec:

[] BUG: KASAN: slab-use-after-free in seqiv_aead_encrypt+0x81a/0x8f0
[] Call Trace:
[]  <TASK>
[]  seqiv_aead_encrypt+0x81a/0x8f0
[]  esp_output_tail+0x706/0x1be0 [esp4]
[]  esp_output+0x4bb/0x9bb [esp4]
[]  xfrm_output_one+0xbac/0x10d0
[]  xfrm_output_resume+0x11e/0xc30
[]  xfrm4_output+0x109/0x460
[]  __ip_queue_xmit+0xc51/0x17f0
[]  __tcp_transmit_skb+0x2555/0x3240
[]  tcp_write_xmit+0x88f/0x3df0
[]  __tcp_push_pending_frames+0x94/0x320
[]  tcp_rcv_established+0x79f/0x3540
[]  tcp_v4_do_rcv+0x4ae/0x8a0
[]  __release_sock+0x29b/0x3b0
[]  release_sock+0x53/0x1d0
[]  tcp_sendmsg+0x35/0x40

[] Allocated by task 7455:
[]  esp_output_tail+0x151/0x1be0 [esp4]
[]  esp_output+0x4bb/0x9bb [esp4]
[]  xfrm_output_one+0xbac/0x10d0
[]  xfrm_output_resume+0x11e/0xc30
[]  xfrm4_output+0x109/0x460
[]  __ip_queue_xmit+0xc51/0x17f0
[]  __tcp_transmit_skb+0x2555/0x3240
[]  tcp_write_xmit+0x88f/0x3df0
[]  __tcp_push_pending_frames+0x94/0x320
[]  tcp_rcv_established+0x79f/0x3540
[]  tcp_v4_do_rcv+0x4ae/0x8a0
[]  __release_sock+0x29b/0x3b0
[]  release_sock+0x53/0x1d0
[]  tcp_sendmsg+0x35/0x40

[] Freed by task 0:
[]  kfree+0x1d5/0x640
[]  esp_output_done+0x43d/0x870 [esp4]
[]  qat_alg_callback+0x83/0xc0 [intel_qat]
[]  adf_ring_response_handler+0x377/0x7f0 [intel_qat]
[]  adf_response_handler+0x66/0x170 [intel_qat]
[]  tasklet_action_common+0x2c9/0x460
[]  handle_softirqs+0x1fd/0x860
[]  __irq_exit_rcu+0xfd/0x250
[]  irq_exit_rcu+0xe/0x30
[]  common_interrupt+0xbc/0xe0
[]  asm_common_interrupt+0x26/0x40

The req allocated in esp_output_tail() may complete asynchronously when
crypto_aead_encrypt() returns -EINPROGRESS or -EBUSY. In this case, the
req can be freed in qat_alg_callback() via esp_output_done(), yet
seqiv_aead_encrypt() still accesses req->iv after the encrypt call
returns:

  if (unlikely(info != req->iv))

There is no guarantee that req remains valid after an asynchronous
submission, and this access will result in this use-after-free.

Fix this by checking req->iv only when the encryption completes
synchronously, and skipping the check for -EINPROGRESS/-EBUSY returns.

Fixes: 856e3f4092cf ("crypto: seqiv - Add support for new AEAD interface")
Reported-by: Xiumei Mu <xmu@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 crypto/seqiv.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/crypto/seqiv.c b/crypto/seqiv.c
index 2bae99e33526..94f6f708ff5f 100644
--- a/crypto/seqiv.c
+++ b/crypto/seqiv.c
@@ -23,9 +23,6 @@ static void seqiv_aead_encrypt_complete2(struct aead_request *req, int err)
 	struct aead_request *subreq = aead_request_ctx(req);
 	struct crypto_aead *geniv;
 
-	if (err == -EINPROGRESS || err == -EBUSY)
-		return;
-
 	if (err)
 		goto out;
 
@@ -40,7 +37,8 @@ static void seqiv_aead_encrypt_complete(void *data, int err)
 {
 	struct aead_request *req = data;
 
-	seqiv_aead_encrypt_complete2(req, err);
+	if (err != -EINPROGRESS && err != -EBUSY)
+		seqiv_aead_encrypt_complete2(req, err);
 	aead_request_complete(req, err);
 }
 
@@ -89,7 +87,7 @@ static int seqiv_aead_encrypt(struct aead_request *req)
 	scatterwalk_map_and_copy(info, req->dst, req->assoclen, ivsize, 1);
 
 	err = crypto_aead_encrypt(subreq);
-	if (unlikely(info != req->iv))
+	if (err != -EINPROGRESS && err != -EBUSY && unlikely(info != req->iv))
 		seqiv_aead_encrypt_complete2(req, err);
 	return err;
 }
-- 
2.47.1

[v2 PATCH] crypto: seqiv - Do not use req->iv after crypto_aead_encrypt

Posted by Herbert Xu 1 month, 3 weeks ago

On Sun, Dec 14, 2025 at 04:42:29PM -0500, Xin Long wrote:
> Xiumei reported a UAF crash when intel_qat is used in ipsec:
> 
> [] BUG: KASAN: slab-use-after-free in seqiv_aead_encrypt+0x81a/0x8f0
> [] Call Trace:
> []  <TASK>
> []  seqiv_aead_encrypt+0x81a/0x8f0
> []  esp_output_tail+0x706/0x1be0 [esp4]
> []  esp_output+0x4bb/0x9bb [esp4]
> []  xfrm_output_one+0xbac/0x10d0
> []  xfrm_output_resume+0x11e/0xc30
> []  xfrm4_output+0x109/0x460
> []  __ip_queue_xmit+0xc51/0x17f0
> []  __tcp_transmit_skb+0x2555/0x3240
> []  tcp_write_xmit+0x88f/0x3df0
> []  __tcp_push_pending_frames+0x94/0x320
> []  tcp_rcv_established+0x79f/0x3540
> []  tcp_v4_do_rcv+0x4ae/0x8a0
> []  __release_sock+0x29b/0x3b0
> []  release_sock+0x53/0x1d0
> []  tcp_sendmsg+0x35/0x40
> 
> [] Allocated by task 7455:
> []  esp_output_tail+0x151/0x1be0 [esp4]
> []  esp_output+0x4bb/0x9bb [esp4]
> []  xfrm_output_one+0xbac/0x10d0
> []  xfrm_output_resume+0x11e/0xc30
> []  xfrm4_output+0x109/0x460
> []  __ip_queue_xmit+0xc51/0x17f0
> []  __tcp_transmit_skb+0x2555/0x3240
> []  tcp_write_xmit+0x88f/0x3df0
> []  __tcp_push_pending_frames+0x94/0x320
> []  tcp_rcv_established+0x79f/0x3540
> []  tcp_v4_do_rcv+0x4ae/0x8a0
> []  __release_sock+0x29b/0x3b0
> []  release_sock+0x53/0x1d0
> []  tcp_sendmsg+0x35/0x40
> 
> [] Freed by task 0:
> []  kfree+0x1d5/0x640
> []  esp_output_done+0x43d/0x870 [esp4]
> []  qat_alg_callback+0x83/0xc0 [intel_qat]
> []  adf_ring_response_handler+0x377/0x7f0 [intel_qat]
> []  adf_response_handler+0x66/0x170 [intel_qat]
> []  tasklet_action_common+0x2c9/0x460
> []  handle_softirqs+0x1fd/0x860
> []  __irq_exit_rcu+0xfd/0x250
> []  irq_exit_rcu+0xe/0x30
> []  common_interrupt+0xbc/0xe0
> []  asm_common_interrupt+0x26/0x40
> 
> The req allocated in esp_output_tail() may complete asynchronously when
> crypto_aead_encrypt() returns -EINPROGRESS or -EBUSY. In this case, the
> req can be freed in qat_alg_callback() via esp_output_done(), yet
> seqiv_aead_encrypt() still accesses req->iv after the encrypt call
> returns:
> 
>   if (unlikely(info != req->iv))
> 
> There is no guarantee that req remains valid after an asynchronous
> submission, and this access will result in this use-after-free.
> 
> Fix this by checking req->iv only when the encryption completes
> synchronously, and skipping the check for -EINPROGRESS/-EBUSY returns.
> 
> Fixes: 856e3f4092cf ("crypto: seqiv - Add support for new AEAD interface")
> Reported-by: Xiumei Mu <xmu@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  crypto/seqiv.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)

Thanks for catching this! I'd prefer not using req->iv at all
though, something like:

---8<---
As soon as crypto_aead_encrypt is called, the underlying request
may be freed by an asynchronous completion.  Thus dereferencing
req->iv after it returns is invalid.

Instead of checking req->iv against info, create a new variable
unaligned_info and use it for that purpose instead.

Fixes: 0a270321dbf9 ("[CRYPTO] seqiv: Add Sequence Number IV Generator")
Reported-by: Xiumei Mu <xmu@redhat.com>
Reported-by: Xin Long <lucien.xin@gmail.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/crypto/seqiv.c b/crypto/seqiv.c
index 2bae99e33526..678bb4145d78 100644
--- a/crypto/seqiv.c
+++ b/crypto/seqiv.c
@@ -50,6 +50,7 @@ static int seqiv_aead_encrypt(struct aead_request *req)
 	struct aead_geniv_ctx *ctx = crypto_aead_ctx(geniv);
 	struct aead_request *subreq = aead_request_ctx(req);
 	crypto_completion_t compl;
+	bool unaligned_info;
 	void *data;
 	u8 *info;
 	unsigned int ivsize = 8;
@@ -68,8 +69,9 @@ static int seqiv_aead_encrypt(struct aead_request *req)
 		memcpy_sglist(req->dst, req->src,
 			      req->assoclen + req->cryptlen);
 
-	if (unlikely(!IS_ALIGNED((unsigned long)info,
-				 crypto_aead_alignmask(geniv) + 1))) {
+	unaligned_info = !IS_ALIGNED((unsigned long)info,
+				     crypto_aead_alignmask(geniv) + 1);
+	if (unlikely(unaligned_info)) {
 		info = kmemdup(req->iv, ivsize, req->base.flags &
 			       CRYPTO_TFM_REQ_MAY_SLEEP ? GFP_KERNEL :
 			       GFP_ATOMIC);
@@ -89,7 +91,7 @@ static int seqiv_aead_encrypt(struct aead_request *req)
 	scatterwalk_map_and_copy(info, req->dst, req->assoclen, ivsize, 1);
 
 	err = crypto_aead_encrypt(subreq);
-	if (unlikely(info != req->iv))
+	if (unlikely(unaligned_info))
 		seqiv_aead_encrypt_complete2(req, err);
 	return err;
 }
-- 
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