Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 12555475 authored by John Garry's avatar John Garry Committed by Greg Kroah-Hartman
Browse files

crypto: hisilicon - Fix reference after free of memories on error path 



commit 0b0cf6af3f3151c26c27e8e51def5527091c3e69 upstream.

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: 915e4e84 ("crypto: hisilicon - SEC security accelerator driver")
Reviewed-by: default avatarJonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: default avatarJohn Garry <john.garry@huawei.com>
Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 093d6ab5

drivers/crypto/hisilicon/sec/sec_algs.c

+11 −10
Original line number Diff line number Diff line
@@ -808,13 +808,6 @@ static int sec_alg_skcipher_crypto(struct skcipher_request *skreq, 
	 * more refined but this is unlikely to happen so no need.

	 */



	/* Cleanup - all elements in pointer arrays have been coppied */

	kfree(splits_in_nents);

	kfree(splits_in);

	kfree(splits_out_nents);

	kfree(splits_out);

	kfree(split_sizes);



	/* Grab a big lock for a long time to avoid concurrency issues */

	mutex_lock(&queue->queuelock);
@@ -829,13 +822,13 @@ static int sec_alg_skcipher_crypto(struct skcipher_request *skreq, 
	     (!queue->havesoftqueue ||

	      kfifo_avail(&queue->softqueue) > steps)) ||

	    !list_empty(&ctx->backlog)) {

		ret = -EBUSY;

		if ((skreq->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG)) {

			list_add_tail(&sec_req->backlog_head, &ctx->backlog);

			mutex_unlock(&queue->queuelock);

			return -EBUSY;

			goto out;

		}



		ret = -EBUSY;

		mutex_unlock(&queue->queuelock);

		goto err_free_elements;

	}
@@ -844,7 +837,15 @@ static int sec_alg_skcipher_crypto(struct skcipher_request *skreq, 
	if (ret)

		goto err_free_elements;



	return -EINPROGRESS;

	ret = -EINPROGRESS;

out:

	/* Cleanup - all elements in pointer arrays have been copied */

	kfree(splits_in_nents);

	kfree(splits_in);

	kfree(splits_out_nents);

	kfree(splits_out);

	kfree(split_sizes);

	return ret;



err_free_elements:

	list_for_each_entry_safe(el, temp, &sec_req->elements, head) {