RE: [PATCH v2 2/2] crypto: virtio: Fix use-after-free in virtio_crypto_skcipher_finalize_req()
From: "Gonglei (Arei)" <arei.gonglei@huawei.com>
Date: 2020-05-26 12:00:44
Also in:
linux-crypto, lkml, stable
-----Original Message-----
From: Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
Sent: Tuesday, May 26, 2020 11:20 AM
To: linux-crypto@vger.kernel.org
Cc: Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
[off-list ref]; LABBE Corentin [off-list ref]; Gonglei
(Arei) [off-list ref]; Herbert Xu
[off-list ref]; Michael S. Tsirkin [off-list ref]; Jason
Wang [off-list ref]; David S. Miller [off-list ref];
Markus Elfring [off-list ref];
virtualization@lists.linux-foundation.org; linux-kernel@vger.kernel.org;
stable@vger.kernel.org
Subject: [PATCH v2 2/2] crypto: virtio: Fix use-after-free in
virtio_crypto_skcipher_finalize_req()
The system'll crash when the users insmod crypto/tcrypto.ko with mode=155
( testing "authenc(hmac(sha1),cbc(aes))" ). It's caused by reuse the memory of
request structure.
In crypto_authenc_init_tfm(), the reqsize is set to:
[PART 1] sizeof(authenc_request_ctx) +
[PART 2] ictx->reqoff +
[PART 3] MAX(ahash part, skcipher part) and the 'PART 3' is used by both
ahash and skcipher in turn.
When the virtio_crypto driver finish skcipher req, it'll call ->complete callback(in
crypto_finalize_skcipher_request) and then free its resources whose pointers
are recorded in 'skcipher parts'.
However, the ->complete is 'crypto_authenc_encrypt_done' in this case, it will
use the 'ahash part' of the request and change its content, so virtio_crypto
driver will get the wrong pointer after ->complete finish and mistakenly free
some other's memory. So the system will crash when these memory will be used
again.
The resources which need to be cleaned up are not used any more. But the
pointers of these resources may be changed in the function
"crypto_finalize_skcipher_request". Thus release specific resources before
calling this function.
Fixes: dbaf0624ffa5 ("crypto: add virtio-crypto driver")
Reported-by: LABBE Corentin <clabbe@baylibre.com>
Cc: Gonglei <arei.gonglei@huawei.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Markus Elfring <redacted>
Cc: virtualization@lists.linux-foundation.org
Cc: linux-kernel@vger.kernel.org
Cc: stable@vger.kernel.org
Message-Id: <20200123101000.GB24255@Red>
Signed-off-by: Longpeng(Mike) <redacted>
---Acked-by: Gonglei <arei.gonglei@huawei.com> Regards, -Gonglei
quoted hunk ↗ jump to hunk
drivers/crypto/virtio/virtio_crypto_algs.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)diff --git a/drivers/crypto/virtio/virtio_crypto_algs.cb/drivers/crypto/virtio/virtio_crypto_algs.c index 5f8243563009..52261b6c247e 100644--- a/drivers/crypto/virtio/virtio_crypto_algs.c +++ b/drivers/crypto/virtio/virtio_crypto_algs.c@@ -582,10 +582,11 @@ static void virtio_crypto_skcipher_finalize_req( scatterwalk_map_and_copy(req->iv, req->dst, req->cryptlen - AES_BLOCK_SIZE, AES_BLOCK_SIZE, 0); - crypto_finalize_skcipher_request(vc_sym_req->base.dataq->engine, - req, err); kzfree(vc_sym_req->iv); virtcrypto_clear_request(&vc_sym_req->base); + + crypto_finalize_skcipher_request(vc_sym_req->base.dataq->engine, + req, err); } static struct virtio_crypto_algo virtio_crypto_algs[] = { { --2.23.0