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

Xin Long posted 1 patch 1 day ago
crypto/seqiv.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
[PATCH] crypto: seqiv: check req->iv only for synchronous completion
Posted by Xin Long 1 day 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