20260720-linuxkm-CMAC-native#10980
Conversation
|
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10980
Scan targets checked: linuxkm-bugs, linuxkm-src, wolfcrypt-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src
Findings: 5
5 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
|
Retest this please. |
3e6a2f1 to
d4c3410
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10980
Scan targets checked: linuxkm-bugs, linuxkm-src, wolfcrypt-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
Frauschi
left a comment
There was a problem hiding this comment.
🐺 Skoll Code Review
Overall recommendation: REQUEST_CHANGES
Findings: 4 total — 2 posted, 2 skipped
Posted findings
- [Medium] enabled_fips declaration/use guard mismatch can break -Werror kernel builds —
linuxkm/lkcapi_glue.c:227 - [Medium] configure.ac lean-aesgcm-dev SHA512 logic inverted from disable to force-enable —
configure.ac:7377-7379
Skipped findings
- [Medium] km_hmac_import() calls allocating wc_HmacCopy() while holding spinlock (sleep-in-atomic)
- [Low] cpuid_flag: reg[num] = 0 is dead code (overwritten by cpuid())
Review generated by Skoll via Claude/Codex
d4c3410 to
5d377ee
Compare
|
retest this please |
Frauschi
left a comment
There was a problem hiding this comment.
🐺 Skoll Code Review
Overall recommendation: REQUEST_CHANGES
Findings: 47 total — 6 posted, 41 skipped
Posted findings
- [High] CMAC transient-copy design breaks under WC_DEBUG_CIPHER_LIFECYCLE: dangling/double-free of Aes.CipherLifecycleTag —
linuxkm/lkcapi_aes_glue.c:5168 - [High] configure.ac lean-aesgcm SHA-512 clause is logically inverted, force-ENABLING SHA-512/SHA-384 in the lean FIPS profile —
configure.ac:7377-7379 - [High] SHA-3 and CMAC shash_algs adopt .init_tfm/.exit_tfm without extending the >= 5.6 kernel-version guard —
linuxkm/lkcapi_sha_glue.c:1177 - [Medium] km_hmac_export reads snapshot->desc_id after releasing the list lock (use-after-free window) —
linuxkm/lkcapi_sha_glue.c:1630-1648 - [Medium] wc_UnLockMutex returns -1 without unlocking on magic mismatch, leaving the spinlock held with IRQs disabled —
linuxkm/linuxkm_wc_port.h:2036-2045 - [Medium] wc_AesGcmInit() WC_C_DYNAMIC_FALLBACK permanently clears aes->use_aesni instead of scoping the fallback to the call —
wolfcrypt/src/aes.c:13993-14008
Skipped findings
- [Medium] km_hmac_import and SHA-3 .init/.import orphan the desc's previous working node onto an uncapped list
- [Medium] km_hmac_import performs wc_HmacCopy (which may allocate) while holding the IRQ-disabling spinlock
- [Medium] WC_SHA256_W_SIZE / WC_SHA512_W_SIZE duplicate private allocation sizes from sha256.c / sha512.c
- [Medium] WC_LINUXKM_SHA2_FREE_W frees an XMALLOC'd buffer with raw free()
- [Medium] The entire SHA-2 stack-W path is compiled out in default kernel builds and therefore ships untested
- [Medium] HMAC .export handle is not self-contained state and is valid only within one tfm and one eviction window
- [Medium] km_hmac_export() reads the snapshot node after publishing it to export_list and dropping the lock (use-after-free read)
- [Medium] New export/import self-tests allocate their tfm by cra_name and assert wolfSSL-private statesize, breaking registration under LINUXKM_LKCAPI_PRIORITY_ALLOW_MASKING
- [Medium] km_hmac_import() orphans a working node per abandoned desc, giving unbounded kernel memory growth on the AF_ALG accept()/close() path
- [Medium] AES-CMAC transient-copy design use-after-frees the Aes CipherLifecycleTag under WC_DEBUG_CIPHER_LIFECYCLE
- [Medium] km_AesCmacSetKey() frees the tfm-owned pristine Cmac that concurrent descs dereference
- [Medium] km_hmac_export() reads snapshot->desc_id after dropping desc_list_lock, racing eviction/free
- [Medium] HMAC export-snapshot ring is sized by CPU count, not in-flight accepts, causing spurious -EINVAL under concurrent AF_ALG accept()
- [Low] SHA-2 one-shot .digest still heap-allocates W, contradicting the stated no-heap-after-init design
- [Low] types.h: undocumented !defined(GNUC) on the C23 static_assert branch changes which implementation GCC/clang select
- [Low] configure.ac 'cmac(aes)' defines HAVE_AES_ECB (unused) instead of WOLFSSL_AES_DIRECT (required by the glue gate)
- [Low] configure.ac lean-aesgcm: SHA-512 force-enable adds -DWOLFSSL_SHA384 while ENABLED_SHA384 stays "no"
- [Low] km_AesCmacFinal/Finup bypass km_AesCmacDispose and bare-free the Cmac
- [Low] SHA-3 one-shot digest places a ~480-byte struct on the kernel stack
- [Low] km_AesCmacSetKey does not fail closed on the key-length rejection path
- [Low] CMAC transient copy does not defensively NULL aes.streamData, unlike km_AesGet() in the same file
- [Low] cpuid.c: replacing XMEMSET of the register array with a single-element zero is unrelated to this PR
- [Low] Bare scope blocks introduced in km_AesCmacFinal / km_AesCmacFinup
- [Low] KMAC-256 FIPS path drops the zero-length-key check instead of asserting KMAC_MIN_KEYLEN_E
- [Low] SHA-3 .init and .import orphan any prior state node onto an unbounded per-TFM desc_list
- [Low] Bounded HMAC export ring makes a previously valid state blob permanently unimportable, so concurrent AF_ALG accept() can fail spuriously
- [Low] km_AesCmacSetKey() does not fail closed on the invalid-key-length path, contradicting its own rekey invariant
- [Low] SHA-3 export blob carries no algorithm/variant tag, permitting cross-variant state import
- [Low] cpuid.c XMEMSET removal not applied to cpuid_is_intel()/cpuid_is_amd(), which run first on the same path
- [Low] km_hmac_import() performs the deep wc_HmacCopy() while holding the IRQ-disabling spinlock
- [Low] wc_FreeMutex() now invalidates the lock, and wc_UnLockMutex() can bail out while still holding the spinlock
- [Low] km_hmac_free_tstate()/km_sha3_free_tstate() silently skip cleanup when the list lock cannot be taken
- [Info] linuxkm_test_aescmac ForceZeros an uninitialized stack struct on early-exit paths
- [Info] wc_FreeMutex uses memset (not XMEMSET/ForceZero) and keeps a now-vestigial (void)m
- [Info] WC_LINUXKM_SPINLOCK_MAGIC is an unexplained decimal literal
- [Info] list_last_entry() without a list_empty() guard, protected only by an assert that can be silently skipped
- [Info] Empty #if/#endif block left behind in the selftest-macro rename
- [Info] settings.h #undefs WOLFSSL_SMALL_STACK before headers are parsed, changing struct layouts inside the sha256.c/sha512.c translation units
- [Info] WC_LINUXKM_SHA2_FREE_W() releases an XMALLOC'd buffer with raw free()
- [Info] WC_LINUXKM_SHA2_PUSH_W/POP_W generate bare brace-scope blocks in every SHA-2 shash entry point
- [Info] SHA-3 .digest places a ~448-byte struct on the stack, contradicting the HMAC digest's stated rationale
Review generated by Skoll via Claude/Codex
…ment linuxkm cmac(aes) glue.
…leaks: * separate WC_LINUXKM_SHA_IMPLEMENT() into WC_LINUXKM_SHA1_IMPLEMENT() (no fixes needed) and WC_LINUXKM_SHA2_IMPLEMENT() (with associated new helpers WC_LINUXKM_SHA2_FREE_W(), WC_LINUXKM_SHA2_DECL_W(), WC_LINUXKM_SHA2_PUSH_W(), and WC_LINUXKM_SHA2_POP_W(), that move .W to a stack buffer). * Reimplement SHA-2 one-shot digest callback to use only direct wolfCrypt calls rather than proxy to other callbacks.
…ecks on kernel struct wolfSSL_Mutex in WOLFSSL_LINUXKM_VERBOSE_DEBUG builds.
…TfmCtx, adding km_sha3_init_tfm(), km_sha3_exit_tfm(), km_sha3_alloc_tstate(), and km_sha3_free_tstate(), implementing garbage collection of abandoned shash_desc objects at .exit_tfm.
…ment km_sha3_export() and per-alg _import() methods, and km_sha3_test_export_import().
* add to struct km_sha_hmac_pstate members desc_list_lock, desc_list, export_list, and export_list_len; * add struct km_sha_hmac_export_state; * add km_hmac_alloc_tstate(), km_hmac_export(), km_hmac_import(), km_hmac_test_export_import(); * and update km_hmac_*() and WC_LINUXKM_HMAC_IMPLEMENT(() to implement.
… conjunctions to fix constant-folding-provoked compiler warnings around fips_enabled; for clarity, rename internal macros WC_LINUXKM_HAVE_SELFTEST and WC_LINUXKM_HAVE_SELFTEST_FULL to WC_LINUX_CONFIG_SELFTESTS and WC_LINUX_CONFIG_SELFTESTS_FULL respectively.
…hten up SHA-3 allocations, avoiding frivolous struct km_sha_state (with inline wc_Sha512) for SHA-3.
* Eliminate pointer from export blob, replacing it with a TFM-specific random cookie and per-desc serial ID. * Fixes kernel address disclosure risk, handle forgery risk, cross-TFM confusion risk, and heap pointer reuse risk.
…sl/wolfcrypt/settings.h: * Revert earlier changes adding WC_SHA256_W_SIZE and WC_SHA512_W_SIZE. * Add WC_SHA2_NO_SMALL_STACK to allow the W work buffer to move back onto the stack. * In wolfssl/wolfcrypt/settings.h, define WC_SHA2_NO_SMALL_STACK by default when WOLFSSL_KERNEL_MODE, and add a clause to #undef WOLFSSL_SMALL_STACK when building the SHA-2 implementations. linuxkm/lkcapi_sha_glue.c: * Refactor out superfluous struct km_sha_state. * Clean up some macro dynamics. * Add static asserts on HASH_MAX_STATESIZE to WC_LINUXKM_SHA1_IMPLEMENT() and WC_LINUXKM_SHA2_IMPLEMENT().
…ntegrity check and DRBG.
… SEGV from gcc misoptimization.
… gating on its declaration (add dependency on defined(CONFIG_CRYPTO_FIPS)); use FIPS_NOT_ALLOWED_E rather than NOT_COMPILED_IN to signal FIPS-forbidden keysizes from linuxkm_test_ecdh_nist_driver() and linuxkm_test_ecdsa_nist_driver() to REGISTER_ALG_OPTIONAL().
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).
5d377ee to
6325c48
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10980
Scan targets checked: linuxkm-bugs, linuxkm-src, wolfcrypt-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src
No new issues found in the changed files. ✅
Implement glue logic for Linux kernel registration of native
cmac(aes)implementation, and fix gaps in several otherstruct shash_algglue implementations:linuxkm/lkcapi_aes_glue.c,linuxkm/lkcapi_glue.c,configure.ac: implement linuxkmcmac(aes)glue.linuxkm/lkcapi_sha_glue.c: fixsha.Wlifecycle management to prevent leaks:separate
WC_LINUXKM_SHA_IMPLEMENT()intoWC_LINUXKM_SHA1_IMPLEMENT()(no fixesneeded) and
WC_LINUXKM_SHA2_IMPLEMENT()(with associated new helpersWC_LINUXKM_SHA2_FREE_W(),WC_LINUXKM_SHA2_DECL_W(),WC_LINUXKM_SHA2_PUSH_W(),and
WC_LINUXKM_SHA2_POP_W(), that move.Wto a stack buffer).Reimplement SHA-2 one-shot digest callback to use only direct wolfCrypt calls
rather than proxy to other callbacks.
linuxkm/linuxkm_wc_port.h,linuxkm/module_hooks.c: implementmagicchecks on kernelstruct wolfSSL_MutexinWOLFSSL_LINUXKM_VERBOSE_DEBUGbuilds.linuxkm/lkcapi_sha_glue.c: refactor SHA-3 state aroundstruct km_Sha3TfmCtx,adding
km_sha3_init_tfm(),km_sha3_exit_tfm(),km_sha3_alloc_tstate(), andkm_sha3_free_tstate(), implementing garbage collection of abandonedshash_descobjects at
.exit_tfm.linuxkm/lkcapi_sha_glue.c: addstruct km_sha3_export_state, and implementkm_sha3_export()and per-alg_import()methods, andkm_sha3_test_export_import().linuxkm/lkcapi_sha_glue.c: fix export/import for HMAC:struct km_sha_hmac_pstatemembersdesc_list_lock,desc_list,export_list, andexport_list_len;struct km_sha_hmac_export_state;km_hmac_alloc_tstate(),km_hmac_export(),km_hmac_import(),km_hmac_test_export_import();km_hmac_*()andWC_LINUXKM_HMAC_IMPLEMENT() to implement.test/:
NO_DH;test_wc_ed448_import_public()andtest_wc_Ed448DecisionCoverage()for FIPS v6;tests/api/test_sha3.cfor KMAC keysize in FIPS builds.tested with