Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

crypto: marvell - properly handle CRYPTO_TFM_REQ_MAY_BACKLOG-flagged requests

The mv_cesa_queue_req() function calls crypto_enqueue_request() to
enqueue a request. In the normal case (i.e the queue isn't full), this
function returns -EINPROGRESS. The current Marvell CESA crypto driver
takes this into account and cleans up the request only if an error
occured, i.e if the return value is not -EINPROGRESS.

Unfortunately this causes problems with
CRYPTO_TFM_REQ_MAY_BACKLOG-flagged requests. When such a request is
passed to crypto_enqueue_request() and the queue is full,
crypto_enqueue_request() will return -EBUSY, but will keep the request
enqueued nonetheless. This situation was not properly handled by the
Marvell CESA driver, which was anyway cleaning up the request in such
a situation. When later on the request was taken out of the backlog
and actually processed, a kernel crash occured due to the internal
driver data structures for this structure having been cleaned up.

To avoid this situation, this commit adds a
mv_cesa_req_needs_cleanup() helper function which indicates if the
request needs to be cleaned up or not after a call to
crypto_enqueue_request(). This helper allows to do the cleanup only in
the appropriate cases, and all call sites of mv_cesa_queue_req() are
fixed to use this new helper function.

Reported-by: Vincent Donnefort <vdonnefort@gmail.com>
Fixes: db509a45339fd ("crypto: marvell/cesa - add TDMA support")
Cc: <stable@vger.kernel.org> # v4.2+
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Tested-by: Vincent Donnefort <vdonnefort@gmail.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

authored by

Thomas Petazzoni and committed by
Herbert Xu
cfcd2271 84cba178

+33 -9
+27
drivers/crypto/marvell/cesa.h
··· 687 687 688 688 int mv_cesa_queue_req(struct crypto_async_request *req); 689 689 690 + /* 691 + * Helper function that indicates whether a crypto request needs to be 692 + * cleaned up or not after being enqueued using mv_cesa_queue_req(). 693 + */ 694 + static inline int mv_cesa_req_needs_cleanup(struct crypto_async_request *req, 695 + int ret) 696 + { 697 + /* 698 + * The queue still had some space, the request was queued 699 + * normally, so there's no need to clean it up. 700 + */ 701 + if (ret == -EINPROGRESS) 702 + return false; 703 + 704 + /* 705 + * The queue had not space left, but since the request is 706 + * flagged with CRYPTO_TFM_REQ_MAY_BACKLOG, it was added to 707 + * the backlog and will be processed later. There's no need to 708 + * clean it up. 709 + */ 710 + if (ret == -EBUSY && req->flags & CRYPTO_TFM_REQ_MAY_BACKLOG) 711 + return false; 712 + 713 + /* Request wasn't queued, we need to clean it up */ 714 + return true; 715 + } 716 + 690 717 /* TDMA functions */ 691 718 692 719 static inline void mv_cesa_req_dma_iter_init(struct mv_cesa_dma_iter *iter,
+3 -4
drivers/crypto/marvell/cipher.c
··· 189 189 { 190 190 struct ablkcipher_request *ablkreq = ablkcipher_request_cast(req); 191 191 struct mv_cesa_ablkcipher_req *creq = ablkcipher_request_ctx(ablkreq); 192 - 193 192 creq->req.base.engine = engine; 194 193 195 194 if (creq->req.base.type == CESA_DMA_REQ) ··· 430 431 return ret; 431 432 432 433 ret = mv_cesa_queue_req(&req->base); 433 - if (ret && ret != -EINPROGRESS) 434 + if (mv_cesa_req_needs_cleanup(&req->base, ret)) 434 435 mv_cesa_ablkcipher_cleanup(req); 435 436 436 437 return ret; ··· 550 551 return ret; 551 552 552 553 ret = mv_cesa_queue_req(&req->base); 553 - if (ret && ret != -EINPROGRESS) 554 + if (mv_cesa_req_needs_cleanup(&req->base, ret)) 554 555 mv_cesa_ablkcipher_cleanup(req); 555 556 556 557 return ret; ··· 692 693 return ret; 693 694 694 695 ret = mv_cesa_queue_req(&req->base); 695 - if (ret && ret != -EINPROGRESS) 696 + if (mv_cesa_req_needs_cleanup(&req->base, ret)) 696 697 mv_cesa_ablkcipher_cleanup(req); 697 698 698 699 return ret;
+3 -5
drivers/crypto/marvell/hash.c
··· 739 739 return 0; 740 740 741 741 ret = mv_cesa_queue_req(&req->base); 742 - if (ret && ret != -EINPROGRESS) { 742 + if (mv_cesa_req_needs_cleanup(&req->base, ret)) 743 743 mv_cesa_ahash_cleanup(req); 744 - return ret; 745 - } 746 744 747 745 return ret; 748 746 } ··· 764 766 return 0; 765 767 766 768 ret = mv_cesa_queue_req(&req->base); 767 - if (ret && ret != -EINPROGRESS) 769 + if (mv_cesa_req_needs_cleanup(&req->base, ret)) 768 770 mv_cesa_ahash_cleanup(req); 769 771 770 772 return ret; ··· 789 791 return 0; 790 792 791 793 ret = mv_cesa_queue_req(&req->base); 792 - if (ret && ret != -EINPROGRESS) 794 + if (mv_cesa_req_needs_cleanup(&req->base, ret)) 793 795 mv_cesa_ahash_cleanup(req); 794 796 795 797 return ret;