Skip to content

Riscv port bug fixes#10864

Merged
douzzer merged 2 commits into
wolfSSL:masterfrom
danielinux:riscv-port-null-checks
Jul 8, 2026
Merged

Riscv port bug fixes#10864
douzzer merged 2 commits into
wolfSSL:masterfrom
danielinux:riscv-port-null-checks

Conversation

@danielinux

@danielinux danielinux commented Jul 8, 2026

Copy link
Copy Markdown
Member

Description

Two bug fixes for RiscV

  1. riscv: return KEYUSAGE_E for AES use without a key schedule (fc1bb63)

    The generic wc_AesEncrypt/wc_AesDecrypt (aes.c) reject an AES object
    whose key schedule was never set (rounds outside 1..7 after halving)
    with KEYUSAGE_E, and every mode inherits that through their int return.
    The RISC-V port's block helpers are void, so nothing reported unkeyed
    use: wc_AesEncryptDirect and wc_AesCcmEncrypt/Decrypt silently
    processed with a garbage schedule, and wc_AesCtrEncrypt classified it
    as BAD_FUNC_ARG instead of KEYUSAGE_E.

    Align the port with the generic error contract:

    • wc_AesEncryptDirect / wc_AesDecryptDirect: add the generic rounds
      check (also fixes wc_CmacUpdate error reporting, which goes through
      wc_AesEncryptDirect on this port)
    • wc_AesCcmEncrypt / wc_AesCcmDecrypt: same check after the argument
      sanity block
    • wc_AesCtrEncrypt (both variants): the existing rounds switch now
      returns KEYUSAGE_E instead of BAD_FUNC_ARG
  2. riscv: add missing argument checks in AES port (7b72cf6)

    Align the RISC-V AES port's argument validation with the generic aes.c
    implementations (and with the port's own vector/scalar-crypto siblings):

    • wc_AesSetKey (base assembly variant, i.e. neither
      WOLFSSL_RISCV_VECTOR_CRYPTO_ASM nor WOLFSSL_RISCV_SCALAR_CRYPTO_ASM):
      reject key == NULL. The vector and scalar-crypto variants of the same
      function already check it; the base variant passed NULL through to the
      key expansion and wc_AesGcmSetKey then dereferenced the uninitialized
      schedule, crashing on e.g. wc_AesGcmSetKey(aes, NULL, 16).

    • wc_AesCcmEncrypt / wc_AesCcmDecrypt: reject authIn == NULL when
      authInSz > 0, as the generic implementation does. The port otherwise
      walks the NULL authIn buffer while computing the CBC-MAC.

Testing

Found by the ISO 26262 per-module MC/DC campaign's argument-matrix tests (test_wc_AesGcmSetKey, test_wc_AesCcmArgMcdc) running the RISC-V port under qemu-riscv64: upstream CI only exercises this port with the KAT suite, which never passes invalid arguments.

Align the RISC-V AES port's argument validation with the generic aes.c
implementations (and with the port's own vector/scalar-crypto siblings):

* wc_AesSetKey (base assembly variant, i.e. neither
  WOLFSSL_RISCV_VECTOR_CRYPTO_ASM nor WOLFSSL_RISCV_SCALAR_CRYPTO_ASM):
  reject key == NULL. The vector and scalar-crypto variants of the same
  function already check it; the base variant passed NULL through to the
  key expansion and wc_AesGcmSetKey then dereferenced the uninitialized
  schedule, crashing on e.g. wc_AesGcmSetKey(aes, NULL, 16).

* wc_AesCcmEncrypt / wc_AesCcmDecrypt: reject authIn == NULL when
  authInSz > 0, as the generic implementation does. The port otherwise
  walks the NULL authIn buffer while computing the CBC-MAC.

Found by the ISO 26262 per-module MC/DC campaign's argument-matrix tests
(test_wc_AesGcmSetKey, test_wc_AesCcmArgMcdc) running the RISC-V port
under qemu-riscv64: upstream CI only exercises this port with the KAT
suite, which never passes invalid arguments.
The generic wc_AesEncrypt/wc_AesDecrypt (aes.c) reject an AES object
whose key schedule was never set (rounds outside 1..7 after halving)
with KEYUSAGE_E, and every mode inherits that through their int return.
The RISC-V port's block helpers are void, so nothing reported unkeyed
use: wc_AesEncryptDirect and wc_AesCcmEncrypt/Decrypt silently
processed with a garbage schedule, and wc_AesCtrEncrypt classified it
as BAD_FUNC_ARG instead of KEYUSAGE_E.

Align the port with the generic error contract:
* wc_AesEncryptDirect / wc_AesDecryptDirect: add the generic rounds
  check (also fixes wc_CmacUpdate error reporting, which goes through
  wc_AesEncryptDirect on this port)
* wc_AesCcmEncrypt / wc_AesCcmDecrypt: same check after the argument
  sanity block
* wc_AesCtrEncrypt (both variants): the existing rounds switch now
  returns KEYUSAGE_E instead of BAD_FUNC_ARG

Found by the ISO 26262 MC/DC campaign argument-matrix tests
(test_wc_AesSetKeyArgMcdc, test_wc_AesModesArgMcdc, test_wc_AesCcmArgMcdc,
test_wc_CmacArgMcdc) under qemu-riscv64.
Copilot AI review requested due to automatic review settings July 8, 2026 17:10
@danielinux danielinux self-assigned this Jul 8, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes RISC-V AES port behavior to align with the generic wolfcrypt/src/aes.c error/argument-validation contract, ensuring invalid key schedule usage and missing argument checks are surfaced consistently instead of silently producing incorrect results or crashes.

Changes:

  • Return KEYUSAGE_E (not BAD_FUNC_ARG) from RISC-V AES-CTR when the AES object has an unusable/unset key schedule (aes->rounds invalid).
  • Add missing key == NULL validation in the base wc_AesSetKey() implementation.
  • Add argument validation for CCM AAD (authIn == NULL when authInSz > 0) and add key-schedule usability checks to direct encrypt/decrypt and CCM paths.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@douzzer douzzer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested with

wolfssl-multi-test.sh ...
cross-riscv64-all-asm
cross-riscv32-all-asm

@douzzer douzzer merged commit a5dbbf8 into wolfSSL:master Jul 8, 2026
320 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants