Skip to content

Commit 6325c48

Browse files
committed
fixes from AI review:
linuxkm/lkcapi_aes_glue.c and linuxkm/lkcapi_sha_glue.c: add CMAC and SHA-3 to algs warned for incompatibility with kernels <5.6; linuxkm/lkcapi_aes_glue.c, wolfcrypt/src/aes.c, wolfcrypt/src/memory.c, and wolfssl/wolfcrypt/memory.h: fix WC_DEBUG_CIPHER_LIFECYCLE in kernel mode (km_AesGet() and km_AesCmacMaterialize()), add WARN_UNUSED_RESULT to wc_debug_CipherLifecycle*(), and fix an unchecked wc_debug_CipherLifecycleFree() in wc_AesFree(); linuxkm/lkcapi_aes_glue.c: fix unsafe wc_CmacFree() call in km_AesCmacSetKey(); linuxkm/lkcapi_sha_glue.c: in km_hmac_export(), avoid unlocked access to snapshot->desc_id (possible UAF under extreme pressure).
1 parent 3af441d commit 6325c48

5 files changed

Lines changed: 63 additions & 16 deletions

File tree

linuxkm/lkcapi_aes_glue.c

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,10 @@
271271
#undef LINUXKM_LKCAPI_REGISTER_AESCMAC
272272
#endif
273273

274+
#if (LINUX_VERSION_CODE < KERNEL_VERSION(5, 6, 0)) && defined(LINUXKM_LKCAPI_REGISTER_AESCMAC)
275+
#error LINUXKM_LKCAPI_REGISTER for AES-CMAC is supported only on Linux kernel versions >= 5.6.0.
276+
#endif
277+
274278
#ifdef LINUXKM_LKCAPI_REGISTER_AESCMAC
275279
#include <wolfssl/wolfcrypt/cmac.h>
276280
#endif
@@ -513,6 +517,16 @@ static int km_AesGet(struct km_AesCtx *ctx, int decrypt_p, int copy_p, Aes **aes
513517
XMEMCPY(aes_copy, ret, sizeof(Aes));
514518
#if defined(WOLFSSL_AESGCM_STREAM) && defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_AESNI)
515519
aes_copy->streamData = NULL;
520+
#endif
521+
#ifdef WC_DEBUG_CIPHER_LIFECYCLE
522+
{
523+
int ret2 = wc_debug_CipherLifecycleInit(&aes_copy->CipherLifecycleTag, NULL);
524+
if (ret2 != 0) {
525+
ForceZero(aes_copy, sizeof *aes_copy);
526+
free(aes_copy);
527+
return -ENOMEM;
528+
}
529+
}
516530
#endif
517531
*aes = aes_copy;
518532
}
@@ -5017,6 +5031,9 @@ static int linuxkm_test_aesecb(void) {
50175031
* configurations: wc_AesFree()'s only mainstream XFREE() target is the
50185032
* GCM-streaming streamData buffer, which the CMAC path never allocates.
50195033
* _CRYPTO_CB per-instance contexts would break that invariant.
5034+
*
5035+
* The tracking allocation when WC_DEBUG_CIPHER_LIFECYCLE is accommodated
5036+
* explicitly in km_AesCmacMaterialize().
50205037
*/
50215038
#if defined(WOLF_CRYPTO_CB) && defined(WOLF_CRYPTO_CB_FREE)
50225039
#error LINUXKM_LKCAPI_REGISTER_AESCMAC is incompatible with WOLF_CRYPTO_CB_FREE.
@@ -5111,14 +5128,16 @@ static int km_AesCmacSetKey(struct crypto_shash *tfm, const u8 *key,
51115128
/* wc_InitCmac() zeroizes the Cmac before use, but doesn't release the
51125129
* embedded Aes on post-wc_AesInit() failures.
51135130
*/
5131+
if (ret != WC_NO_ERR_TRACE(BAD_FUNC_ARG)) {
51145132
#ifdef WC_LINUXKM_CMAC_HAVE_CMACFREE
5115-
(void)wc_CmacFree(new_pristine);
5133+
(void)wc_CmacFree(new_pristine);
51165134
#else
5117-
/* no wc_CmacFree() in the boundary -- a failed-init Cmac holds no
5118-
* allocations in pre-v6 FIPS configurations, so zeroization suffices.
5119-
*/
5120-
ForceZero(new_pristine, sizeof(*new_pristine));
5135+
/* no wc_CmacFree() in the boundary -- a failed-init Cmac holds no
5136+
* allocations in pre-v6 FIPS configurations, so zeroization suffices.
5137+
*/
5138+
ForceZero(new_pristine, sizeof(*new_pristine));
51215139
#endif
5140+
}
51225141
free(new_pristine);
51235142
/* fail closed on rekey failure -- the kernel re-flags NEED_KEY, and a
51245143
* stale pristine would misattribute subsequent traffic to the old
@@ -5166,6 +5185,16 @@ static int km_AesCmacMaterialize(struct crypto_shash *tfm,
51665185
return -ENOMEM;
51675186

51685187
XMEMCPY(cmac, t_ctx->pristine, sizeof(*cmac));
5188+
#ifdef WC_DEBUG_CIPHER_LIFECYCLE
5189+
{
5190+
int ret = wc_debug_CipherLifecycleInit(&cmac->aes.CipherLifecycleTag, NULL);
5191+
if (ret != 0) {
5192+
ForceZero(cmac, sizeof *cmac);
5193+
free(cmac);
5194+
return -ENOMEM;
5195+
}
5196+
}
5197+
#endif
51695198

51705199
XMEMCPY(cmac->digest, st->digest, WC_AES_BLOCK_SIZE);
51715200
XMEMCPY(cmac->buffer, st->buffer, WC_AES_BLOCK_SIZE);

linuxkm/lkcapi_sha_glue.c

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,11 @@
407407
#endif
408408

409409
#if (LINUX_VERSION_CODE < KERNEL_VERSION(5, 6, 0)) && \
410-
(defined(LINUXKM_LKCAPI_REGISTER_SHA1_HMAC) || \
410+
(defined(LINUXKM_LKCAPI_REGISTER_SHA3_224) || \
411+
defined(LINUXKM_LKCAPI_REGISTER_SHA3_256) || \
412+
defined(LINUXKM_LKCAPI_REGISTER_SHA3_384) || \
413+
defined(LINUXKM_LKCAPI_REGISTER_SHA3_512) || \
414+
defined(LINUXKM_LKCAPI_REGISTER_SHA1_HMAC) || \
411415
defined(LINUXKM_LKCAPI_REGISTER_SHA2_224_HMAC) || \
412416
defined(LINUXKM_LKCAPI_REGISTER_SHA2_256_HMAC) || \
413417
defined(LINUXKM_LKCAPI_REGISTER_SHA2_384_HMAC) || \
@@ -416,7 +420,7 @@
416420
defined(LINUXKM_LKCAPI_REGISTER_SHA3_256_HMAC) || \
417421
defined(LINUXKM_LKCAPI_REGISTER_SHA3_384_HMAC) || \
418422
defined(LINUXKM_LKCAPI_REGISTER_SHA3_512_HMAC))
419-
#error LINUXKM_LKCAPI_REGISTER for HMACs is supported only on Linux kernel versions >= 5.6.0.
423+
#error LINUXKM_LKCAPI_REGISTER for SHA-3 and HMACs is supported only on Linux kernel versions >= 5.6.0.
420424
#endif
421425

422426
#ifdef HAVE_HASHDRBG
@@ -1588,6 +1592,7 @@ WC_MAYBE_UNUSED static int km_hmac_export(struct shash_desc *desc, void *out)
15881592
struct km_sha_hmac_node *snapshot;
15891593
struct km_sha_hmac_node *evicted = NULL;
15901594
int ret;
1595+
typeof(snapshot->desc_id) snapshot_desc_id;
15911596

15921597
if (s_ctx->node == NULL)
15931598
return -EINVAL;
@@ -1624,7 +1629,7 @@ WC_MAYBE_UNUSED static int km_hmac_export(struct shash_desc *desc, void *out)
16241629
list_del(&evicted->desc_ent);
16251630
p_ctx->export_list_len--;
16261631
}
1627-
snapshot->desc_id = p_ctx->cur_desc_id++;
1632+
snapshot_desc_id = snapshot->desc_id = p_ctx->cur_desc_id++;
16281633
/* list_add() prepends, so the tail from list_last_entry() is the oldest. */
16291634
list_add(&snapshot->desc_ent, &p_ctx->export_list);
16301635
p_ctx->export_list_len++;
@@ -1645,7 +1650,7 @@ WC_MAYBE_UNUSED static int km_hmac_export(struct shash_desc *desc, void *out)
16451650
XMEMSET(blob, 0, sizeof(*blob));
16461651
blob->magic = WC_LINUXKM_HMAC_EXPORT_MAGIC;
16471652
blob->tfm_cookie = p_ctx->tfm_cookie;
1648-
blob->desc_id = snapshot->desc_id;
1653+
blob->desc_id = snapshot_desc_id;
16491654

16501655
return 0;
16511656
}

wolfcrypt/src/aes.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15550,7 +15550,11 @@ void wc_AesFree(Aes* aes)
1555015550
#endif /* WOLF_CRYPTO_CB && WOLF_CRYPTO_CB_FREE */
1555115551

1555215552
#ifdef WC_DEBUG_CIPHER_LIFECYCLE
15553-
(void)wc_debug_CipherLifecycleFree(&aes->CipherLifecycleTag, aes->heap, 1);
15553+
{
15554+
int ret = wc_debug_CipherLifecycleFree(&aes->CipherLifecycleTag, aes->heap, 1);
15555+
if (ret != 0)
15556+
WOLFSSL_DEBUG_PRINTF("ERROR: wc_AesFree(): wc_debug_CipherLifecycleFree() returned %d.\n", ret);
15557+
}
1555415558
#endif
1555515559

1555615560
#if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_AES)

wolfcrypt/src/memory.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1735,8 +1735,12 @@ WOLFSSL_LOCAL int wc_debug_CipherLifecycleCheck(
17351735
ret = 0;
17361736

17371737
out:
1738+
#ifdef WOLFSSL_KERNEL_MODE
1739+
(void)abort_p;
1740+
#else
17381741
if ((ret < 0) && abort_p)
17391742
abort();
1743+
#endif
17401744

17411745
return ret;
17421746
}

wolfssl/wolfcrypt/memory.h

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -340,12 +340,17 @@ WOLFSSL_API int wc_ConstantCompare(const byte* a, const byte* b, int length);
340340
#endif
341341

342342
#ifdef WC_DEBUG_CIPHER_LIFECYCLE
343-
WOLFSSL_LOCAL int wc_debug_CipherLifecycleInit(void **CipherLifecycleTag,
344-
void *heap);
345-
WOLFSSL_LOCAL int wc_debug_CipherLifecycleCheck(void *CipherLifecycleTag,
346-
int abort_p);
347-
WOLFSSL_LOCAL int wc_debug_CipherLifecycleFree(void **CipherLifecycleTag,
348-
void *heap, int abort_p);
343+
#if !defined(HAVE_FIPS) || FIPS_VERSION3_GE(7,0,0)
344+
#define WC_DEBUG_CIPHERLIFECYCLE_WUR WARN_UNUSED_RESULT
345+
#else
346+
#define WC_DEBUG_CIPHERLIFECYCLE_WUR
347+
#endif
348+
WOLFSSL_LOCAL WC_DEBUG_CIPHERLIFECYCLE_WUR int wc_debug_CipherLifecycleInit
349+
(void **CipherLifecycleTag, void *heap);
350+
WOLFSSL_LOCAL WC_DEBUG_CIPHERLIFECYCLE_WUR int wc_debug_CipherLifecycleCheck
351+
(void *CipherLifecycleTag, int abort_p);
352+
WOLFSSL_LOCAL WC_DEBUG_CIPHERLIFECYCLE_WUR int wc_debug_CipherLifecycleFree
353+
(void **CipherLifecycleTag, void *heap, int abort_p);
349354
#else
350355
#define wc_debug_CipherLifecycleInit(CipherLifecycleTag, heap) \
351356
((void)(CipherLifecycleTag), (void)(heap), 0)

0 commit comments

Comments
 (0)