Harden QNX CAAM memory handling by aidangarske · Pull Request #11346 · wolfSSL/wolfssl

aidangarske · 2026-09-01T18:00:03Z

 F-12309, F-12312, F-12308, F-12310, F-12311

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

Hardens QNX CAAM secure-memory handling by tightening partition bounds/ownership checks, hardening request length validation, and adding regression tests to catch oversized/partial request issues.

Changes:

Add QNX partition bounds helpers and enforce partition ownership checks for READ/WRITE/FREE flows.
Harden request parsing: validate key modifier length for BLOB, validate AES request sizes and short reads, and clear local shared buffers.
Add QNX-focused regression tests (shell harness + QNX/host buildable C test scaffolding).

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
wolfssl/wolfcrypt/port/caam/caam_driver.h	Adds QNX partition count constants and a validation macro used by QNX hardening.
wolfcrypt/src/port/caam/caam_qnx.c	Adds zeroization helper, stronger size/range checks, and enforces partition ownership for secure memory operations.
wolfcrypt/src/port/caam/caam_driver.c	Validates partition indices against hardware-reported partition count in QNX builds.
tests/include.am	Ships an additional QNX CAAM regression script in test artifacts.
tests/caam_qnx_blob.test	Adds a portable harness that regression-tests the BLOB key modifier bounds check.
IDE/QNX/CAAM-DRIVER/test_support/*	Adds host stubs/shims to compile QNX CAAM server code for regression testing off-target.
IDE/QNX/CAAM-DRIVER/test_aes_request_length.c	Adds regression coverage for AES short-read, oversize arithmetic, and partition validation/ownership logic.
IDE/QNX/CAAM-DRIVER/Makefile	Adds convenience targets to build/run the new AES request-length regression on host and target.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #11346

Scan targets checked: wolfcrypt-port-bugs, wolfcrypt-rs-bugs, wolfssl-bugs, wolfssl-src

Findings: 3
3 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

github-actions · 2026-09-02T02:08:48Z

stale review

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #11346

Scan targets checked: wolfcrypt-port-bugs, wolfcrypt-rs-bugs, wolfssl-bugs, wolfssl-src
Findings: 5
4 finding(s) posted as inline comments (see file-level comments below)

Required changes (1)

Secure-memory ECDSA operations bypass new ownership tracking

File: wolfcrypt/src/port/caam/caam_qnx.c:1116
Function: doECDSA_KEYPAIR / io_devctl
Category: Cryptographic correctness

doECDSA_KEYPAIR() records an OCB owner, but SM-backed sign and ECDH operations never check it. Another client can enumerate the 16 page addresses and use another client's private key. Adjacent to #12310: these are crypto-use paths.

Related known finding #12310 (similar but distinct): Both omit QNX secure-memory ownership enforcement and can expose another client's key material, but #12310 covers read, write, and free partition handlers whereas this covers ECDSA sign/ECDH key-use paths. The faulting operations and checks to add differ, so one patch would not fix both.

Suggested fix: Record the keypair's page mapping, pass ocb to sign/ECDH handlers, and reject SM key addresses not owned by that OCB.
Basis: The PR's ownership invariant binds secure-memory partitions to the requesting iofunc_ocb_t * before permitting partition access.

This review was generated automatically by Fenrir. Reported findings require changes before merge.

wolfSSL-Fenrir-bot · 2026-09-09T00:35:04Z

    }

+    if (ret == EOK && args[0] == CAAM_BLACK_KEY_SM &&
+            !CAAM_QNX_PARTITION_IS_VALID(args[2])) {


ECDSA ownership confuses page and partition indices · Logic errors

caamECDSAMake() returns the allocated page in args[2], but the new validation and owner claim treat it as a partition. When they differ, ownership is overwritten and cleanup frees the wrong partition; unlike known #12309, the index is in bounds.

Related known finding #12309 (similar but distinct): Both involve QNX secure-memory ownership-table indexing in doECDSA_KEYPAIR, but #12309 is an unchecked client partition index causing out-of-bounds access, while this swaps valid page and partition identities and corrupts ownership/cleanup. Bounds validation and preserving separate indices require different patches.

Suggested fix: Keep separate partition and page variables in caamECDSAMake() and return the original partition—not the allocated page—in args[2].

wolfSSL-Fenrir-bot · 2026-09-09T00:35:04Z

    if (useLocalMem) {
        /* done using local mapped memory */
+        if (key != NULL)
+            caamZeroMemory(key, (size_t)expectedReadSz);


AES fallback is wiped after being unmapped · Logic errors

When startup leaves localMemory == NULL, sem_trywait() still sets useLocalMem and doAES() falls back to pt. Cleanup unmaps pt before the new second wipe of key, dereferencing unmapped memory; unlike #11230, this path owns the semaphore.

Related known finding #11230 (similar but distinct): Both are doAES local-memory/semaphore cleanup paths, but #11230 posts the semaphore without acquiring it after a fallback-map failure, while this selects a fallback when localMemory is NULL and wipes it after unmapping. Semaphore-ownership tracking and fallback-selection/lifetime fixes are independent.

Suggested fix: Require non-NULL localMemory before setting useLocalMem, and keep it clear when using a per-request fallback mapping.

wolfSSL-Fenrir-bot · 2026-09-09T00:35:04Z

+
+    pageNumber = (unsigned int)((partAddr - CAAM_PAGE) / CAAM_PAGE_SZ);
+    if (!CAAM_QNX_PARTITION_IS_VALID(pageNumber) ||
+            (sm_pagePart[pageNumber] != NO_OWNER_PART &&


Partition collision is detected after reassignment · Resource leaks on error paths

doGET_PART() checks sm_pagePart only after caamGetPartition() has reassigned the hardware page. A collision returns without rollback, leaving the new partition untracked and invalidating the prior allocation; unlike #12310, this is post-allocation bookkeeping.

Related known finding #12309 (similar but distinct): Both affect doGET_PART secure-memory partition bookkeeping, but #12309 indexes owner state with an unchecked client-supplied partition number, while this detects a page collision only after hardware reassignment and fails to roll it back. Index validation and atomic allocation/bookkeeping require different patches.

Suggested fix: Prevent allocation from selecting a tracked page, and commit the hardware allocation and sm_pagePart state as one operation.

wolfSSL-Fenrir-bot · 2026-09-09T00:35:04Z

    }

    if (pt != NULL) {
+        caamZeroMemory(key, (size_t)expectedReadSz);


AES cleanup leaves decrypted output in scratch memory · Missing ForceZero

doAES() wipes only expectedReadSz, excluding the adjacent outSz region. Decrypted plaintext remains in persistent localMemory and released mappings. Adjacent to #12312: this is output cleanup, not short-read acceptance.

Related known finding #12312 (similar but distinct): Both are doAES persistent scratch-memory confidentiality issues, but #12312 accepts short input reads and processes stale preexisting bytes, while this fails to wipe the produced output region during cleanup. Exact-read enforcement and complete post-operation wiping are separate fixes.

Suggested fix: Wipe totalSz bytes after output delivery at both cleanup sites; keep the pre-read wipe limited to expectedReadSz.
Basis: ISO/IEC 9899:2018 §6.5.6 permits traversing the complete allocated array object; doAES() allocates that object with totalSz bytes.

aidangarske self-assigned this Sep 1, 2026

Copilot AI lite review requested due to automatic review settings September 1, 2026 18:00

Copilot AI reviewed Sep 1, 2026

View reviewed changes

Comment thread tests/include.am

Comment thread wolfcrypt/src/port/caam/caam_driver.c Outdated

Comment thread wolfcrypt/src/port/caam/caam_qnx.c

Comment thread wolfcrypt/src/port/caam/caam_qnx.c

Comment thread IDE/QNX/CAAM-DRIVER/Makefile

Comment thread tests/caam_qnx_blob.test

aidangarske requested a review from wolfSSL-Fenrir-bot September 1, 2026 18:37

Copilot started reviewing on behalf of aidangarske September 1, 2026 19:03 View session

wolfSSL-Fenrir-bot previously requested changes Sep 1, 2026

View reviewed changes

aidangarske force-pushed the fenrir-fixes-12309-12312-12310-12311 branch from 9d5ed74 to c3b3474 Compare September 7, 2026 23:32

aidangarske added 7 commits September 8, 2026 13:50

F-12309 - Validate QNX secure-memory partition indices

b2f0d44

F-12312 - Reject short QNX AES requests

c680d97

F-12308 - Bound QNX CAAM BLOB key modifier input

73cd0f4

F-12310 - Enforce QNX partition ownership

94955cc

F-12311 - Validate QNX partition access ranges

0fca7c3

Fix QNX CAAM review findings

d68f22c

Fix QNX CAAM partition ownership tracking

cc4bc1b

aidangarske force-pushed the fenrir-fixes-12309-12312-12310-12311 branch from c3b3474 to cc4bc1b Compare September 8, 2026 20:52

aidangarske requested a review from philljj September 8, 2026 22:59

aidangarske assigned wolfSSL-Bot and unassigned aidangarske Sep 8, 2026

philljj requested a review from wolfSSL-Fenrir-bot September 9, 2026 00:01

wolfSSL-Fenrir-bot requested changes Sep 9, 2026

View reviewed changes

Navigation Menu

Uh oh!

Harden QNX CAAM memory handling - #11346

Harden QNX CAAM memory handling#11346
aidangarske wants to merge 7 commits into
wolfSSL:masterfrom
aidangarske:fenrir-fixes-12309-12312-12310-12311

aidangarske commented Sep 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Uh oh!

github-actions Bot commented Sep 2, 2026 •

edited

Loading

Uh oh!

wolfSSL-Fenrir-bot left a comment

Uh oh!

wolfSSL-Fenrir-bot Sep 9, 2026

Uh oh!

wolfSSL-Fenrir-bot Sep 9, 2026

Uh oh!

wolfSSL-Fenrir-bot Sep 9, 2026

Uh oh!

wolfSSL-Fenrir-bot Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

aidangarske commented Sep 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #11346

Uh oh!

github-actions Bot commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

MemBrowse Memory Report

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #11346

Required changes (1)

Secure-memory ECDSA operations bypass new ownership tracking

Uh oh!

wolfSSL-Fenrir-bot Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

wolfSSL-Fenrir-bot Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

wolfSSL-Fenrir-bot Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

wolfSSL-Fenrir-bot Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Sep 2, 2026 •

edited

Loading