Skip to content

Harden QNX CAAM memory handling - #11346

Open
aidangarske wants to merge 7 commits into
wolfSSL:masterfrom
aidangarske:fenrir-fixes-12309-12312-12310-12311
Open

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

Conversation

@aidangarske

Copy link
Copy Markdown
Member
 F-12309, F-12312, F-12308, F-12310, F-12311

@aidangarske aidangarske self-assigned this Sep 1, 2026
Copilot AI lite review requested due to automatic review settings September 1, 2026 18:00

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.

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.

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

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

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

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

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

@philljj philljj 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.

fenrir items looks straightforward, please review

@philljj philljj assigned aidangarske and unassigned wolfSSL-Bot Sep 9, 2026
@aidangarske

aidangarske commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

Jenkins retest this please
(apple m1 flake)

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.

5 participants