Re: [PATCH] crypto: qat - fix out-of-bounds read

Hi, Giovanni, all,

On Wed, Feb 1, 2023 at 4:59 PM Giovanni Cabiddu giovanni.cabiddu@intel.com 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@vger.kernel.org Reported-by: Vladis Dronov vdronov@redhat.com Signed-off-by: Giovanni Cabiddu giovanni.cabiddu@intel.com Reviewed-by: Fiona Trahe fiona.trahe@intel.com

---

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@redhat.com Tested-by: Vladis Dronov vdronov@redhat.com

Best regards, Vladis Dronov | Red Hat, Inc. | The Core Kernel | Senior Software Engineer