Skip to content

Commit 0b0cf6a

Browse files
John Garryherbertx
John Garry
authored andcommitted
crypto: hisilicon - Fix reference after free of memories on error path
coccicheck currently warns of the following issues in the driver: drivers/crypto/hisilicon/sec/sec_algs.c:864:51-66: ERROR: reference preceded by free on line 812 drivers/crypto/hisilicon/sec/sec_algs.c:864:40-49: ERROR: reference preceded by free on line 813 drivers/crypto/hisilicon/sec/sec_algs.c:861:8-24: ERROR: reference preceded by free on line 814 drivers/crypto/hisilicon/sec/sec_algs.c:860:41-51: ERROR: reference preceded by free on line 815 drivers/crypto/hisilicon/sec/sec_algs.c:867:7-18: ERROR: reference preceded by free on line 816 It would appear than on certain error paths that we may attempt reference- after-free some memories. This patch fixes those issues. The solution doesn't look perfect, but having same memories free'd possibly from separate functions makes it tricky. Fixes: 915e4e8 ("crypto: hisilicon - SEC security accelerator driver") Reviewed-by: Jonathan Cameron <[email protected]> Cc: <[email protected]> Signed-off-by: John Garry <[email protected]> Signed-off-by: Herbert Xu <[email protected]>
1 parent 68a031d commit 0b0cf6a

File tree

1 file changed

+11
-10
lines changed

1 file changed

+11
-10
lines changed

drivers/crypto/hisilicon/sec/sec_algs.c

+11-10
Original file line numberDiff line numberDiff line change
@@ -808,13 +808,6 @@ static int sec_alg_skcipher_crypto(struct skcipher_request *skreq,
808808
* more refined but this is unlikely to happen so no need.
809809
*/
810810

811-
/* Cleanup - all elements in pointer arrays have been coppied */
812-
kfree(splits_in_nents);
813-
kfree(splits_in);
814-
kfree(splits_out_nents);
815-
kfree(splits_out);
816-
kfree(split_sizes);
817-
818811
/* Grab a big lock for a long time to avoid concurrency issues */
819812
mutex_lock(&queue->queuelock);
820813

@@ -829,13 +822,13 @@ static int sec_alg_skcipher_crypto(struct skcipher_request *skreq,
829822
(!queue->havesoftqueue ||
830823
kfifo_avail(&queue->softqueue) > steps)) ||
831824
!list_empty(&ctx->backlog)) {
825+
ret = -EBUSY;
832826
if ((skreq->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG)) {
833827
list_add_tail(&sec_req->backlog_head, &ctx->backlog);
834828
mutex_unlock(&queue->queuelock);
835-
return -EBUSY;
829+
goto out;
836830
}
837831

838-
ret = -EBUSY;
839832
mutex_unlock(&queue->queuelock);
840833
goto err_free_elements;
841834
}
@@ -844,7 +837,15 @@ static int sec_alg_skcipher_crypto(struct skcipher_request *skreq,
844837
if (ret)
845838
goto err_free_elements;
846839

847-
return -EINPROGRESS;
840+
ret = -EINPROGRESS;
841+
out:
842+
/* Cleanup - all elements in pointer arrays have been copied */
843+
kfree(splits_in_nents);
844+
kfree(splits_in);
845+
kfree(splits_out_nents);
846+
kfree(splits_out);
847+
kfree(split_sizes);
848+
return ret;
848849

849850
err_free_elements:
850851
list_for_each_entry_safe(el, temp, &sec_req->elements, head) {

0 commit comments

Comments
 (0)