[PATCH] crypto: hisilicon/sec - Fix double free of split scatterlists

[ZhaoJinming]

sec_alg_skcipher_crypto() splits the request scatterlists into per-element
lists with sec_map_and_split_sg().  Each element keeps a reference to its
share in el->sgl_in/el->sgl_out, and sec_alg_free_el() releases them when
the element is torn down.

The error paths that jump to err_free_elements free the elements with
sec_alg_free_el(), which kfree()s el->sgl_in and el->sgl_out.  Execution
then falls through into err_unmap_out_sg/err_unmap_in_sg, where
sec_unmap_sg_on_err() kfree()s the very same splits[] pointers again, so
every scatterlist that was already attached to an element is freed twice.
This is reachable through the queue-full back-pressure path taken when the
request is not marked CRYPTO_TFM_REQ_MAY_BACKLOG, through a failed
sec_send_request(), or through a failed sec_alg_alloc_and_fill_el().

Hand ownership of the split scatterlists over to the element as soon as it
is created by clearing the matching entries in the splits_in/splits_out
arrays.  Entries that were never assigned to an element are still released
by sec_unmap_sg_on_err().

Fixes: 915e4e8413da ("crypto: hisilicon - SEC security accelerator driver")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: ZhaoJinming <zhaojinming@xxxxxxxxxxxxx>
---
The per-element split scatterlists are released by sec_alg_free_el() and
then freed a second time by sec_unmap_sg_on_err() on the error path that
falls through from err_free_elements.  The bug has been present since the
driver was introduced in 915e4e8413da and was not covered by the earlier
error-path fix 0b0cf6af3f31, which only handled split_sizes and the
temporary pointer arrays.
---
 drivers/crypto/hisilicon/sec/sec_algs.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/crypto/hisilicon/sec/sec_algs.c b/drivers/crypto/hisilicon/sec/sec_algs.c
index 85eecbb40e7e..fa9ccd8109f3 100644
--- a/drivers/crypto/hisilicon/sec/sec_algs.c
+++ b/drivers/crypto/hisilicon/sec/sec_algs.c
@@ -782,6 +782,15 @@ static int sec_alg_skcipher_crypto(struct skcipher_request *skreq,
 			ret = PTR_ERR(el);
 			goto err_free_elements;
 		}
+		/*
+		 * The element owns the split scatterlists from here on and
+		 * releases them in sec_alg_free_el().  Drop the references
+		 * kept in the temporary pointer arrays so that the error
+		 * cleanup does not kfree() the same memory a second time.
+		 */
+		splits_in[i] = NULL;
+		if (split)
+			splits_out[i] = NULL;
 		el->req.cipher_iv_addr_lo = lower_32_bits(sec_req->dma_iv);
 		el->req.cipher_iv_addr_hi = upper_32_bits(sec_req->dma_iv);
 		el->sec_req = sec_req;

---
base-commit: 3857c2fe5449541e24afc5efdb0f81a8a8f9a3a0
change-id: 20261010-hisilicon-sec-double-free-7e8080612310

Best regards,
--  
ZhaoJinming <zhaojinming@xxxxxxxxxxxxx>