Hi, Giovanni, all,
On Wed, Feb 1, 2023 at 4:59 PM Giovanni Cabiddu
<giovanni.cabiddu@xxxxxxxxx> wrote:
>
> When preparing an AER-CTR request, the driver copies the key provided by
> the user into a data structure that is accessible by the firmware.
> If the target device is QAT GEN4, the key size is rounded up by 16 since
> a rounded up size is expected by the device.
> If the key size is rounded up before the copy, the size used for copying
> the key might be bigger than the size of the region containing the key,
> causing an out-of-bounds read.
>
> Fix by doing the copy first and then update the keylen.
>
> This is to fix the following warning reported by KASAN:
>
> [ 138.150574] BUG: KASAN: global-out-of-bounds in qat_alg_skcipher_init_com.isra.0+0x197/0x250 [intel_qat]
> [ 138.150641] Read of size 32 at addr ffffffff88c402c0 by task cryptomgr_test/2340
>
> [ 138.150651] CPU: 15 PID: 2340 Comm: cryptomgr_test Not tainted 6.2.0-rc1+ #45
> [ 138.150659] Hardware name: Intel Corporation ArcherCity/ArcherCity, BIOS EGSDCRB1.86B.0087.D13.2208261706 08/26/2022
> [ 138.150663] Call Trace:
> [ 138.150668] <TASK>
> [ 138.150922] kasan_check_range+0x13a/0x1c0
> [ 138.150931] memcpy+0x1f/0x60
> [ 138.150940] qat_alg_skcipher_init_com.isra.0+0x197/0x250 [intel_qat]
> [ 138.151006] qat_alg_skcipher_init_sessions+0xc1/0x240 [intel_qat]
> [ 138.151073] crypto_skcipher_setkey+0x82/0x160
> [ 138.151085] ? prepare_keybuf+0xa2/0xd0
> [ 138.151095] test_skcipher_vec_cfg+0x2b8/0x800
>
> Fixes: 67916c951689 ("crypto: qat - add AES-CTR support for QAT GEN4 devices")
> Cc: <stable@xxxxxxxxxxxxxxx>
> Reported-by: Vladis Dronov <vdronov@xxxxxxxxxx>
> Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@xxxxxxxxx>
> Reviewed-by: Fiona Trahe <fiona.trahe@xxxxxxxxx>
> ---
> drivers/crypto/qat/qat_common/qat_algs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/crypto/qat/qat_common/qat_algs.c b/drivers/crypto/qat/qat_common/qat_algs.c
> index b4b9f0aa59b9..b61ada559158 100644
> --- a/drivers/crypto/qat/qat_common/qat_algs.c
> +++ b/drivers/crypto/qat/qat_common/qat_algs.c
> @@ -435,8 +435,8 @@ static void qat_alg_skcipher_init_com(struct qat_alg_skcipher_ctx *ctx,
> } else if (aes_v2_capable && mode == ICP_QAT_HW_CIPHER_CTR_MODE) {
> ICP_QAT_FW_LA_SLICE_TYPE_SET(header->serv_specif_flags,
> ICP_QAT_FW_LA_USE_UCS_SLICE_TYPE);
> - keylen = round_up(keylen, 16);
> memcpy(cd->ucs_aes.key, key, keylen);
> + keylen = round_up(keylen, 16);
> } else {
> memcpy(cd->aes.key, key, keylen);
> }
> --
> 2.39.1
>
Thanks, the fix seems to be working. It was tested on "Intel(R) Xeon(R)
Platinum 8468H / Sapphire Rapids 4 skt (SPR) XCC-SP, Qual E-3
stepping" machine with 8086:4940 (rev 40) QAT device:
Without the fix:
# dmesg | grep KASAN
[ 142.612847] BUG: KASAN: global-out-of-bounds in
qat_alg_skcipher_init_com.isra.0+0x2fe/0x440 [intel_qat]
With the fix:
# dmesg | grep KASAN
<no output>
So,
Reviewed-by: Vladis Dronov <vdronov@xxxxxxxxxx>
Tested-by: Vladis Dronov <vdronov@xxxxxxxxxx>
Best regards,
Vladis Dronov | Red Hat, Inc. | The Core Kernel | Senior Software Engineer
