Skip to content

Fix IAR warning about volatile access#10045

Merged
douzzer merged 2 commits intowolfSSL:masterfrom
embhorn:zd21385
Mar 30, 2026
Merged

Fix IAR warning about volatile access#10045
douzzer merged 2 commits intowolfSSL:masterfrom
embhorn:zd21385

Conversation

@embhorn
Copy link
Copy Markdown
Member

@embhorn embhorn commented Mar 23, 2026

Description

Read the volatile once into a local copy, then use the copy in expressions — satisfying IAR's stricter volatile access rules. "undefined behavior: the order of volatile accesses is undefined in this statement"

Fixes zd21385

Testing

Customer confirmed clean build with IAR tools

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@embhorn embhorn self-assigned this Mar 23, 2026
Copilot AI review requested due to automatic review settings March 23, 2026 13:24
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates RSA unpadding and ticket decryption error masking to satisfy IAR’s stricter rules around volatile access by first reading volatile values into local copies and using those in subsequent expressions.

Changes:

  • Refactored constant-time RSA unpadding checks to use local copies of volatile variables in expressions.
  • Updated ticket encryption callback RSA path to use a local copy of a volatile mask when composing lastErr.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
wolfcrypt/src/rsa.c Reads volatile masking state into local temporaries before combining/deriving masks to avoid IAR volatile-use warnings.
src/internal.c Uses a local copy of lenErrMask when building args->lastErr to avoid volatile access in compound expressions.
Comments suppressed due to low confidence (2)

src/internal.c:1

  • Since lenErrMaskCopy is the value actually used in subsequent expressions, computing directly into the non-volatile local (and only keeping volatile if it’s required for the toolchain workaround) would reduce noise and avoid an extra volatile write+read pair. For example, compute the mask into lenErrMaskCopy and use that consistently.
    src/internal.c:1
  • Since lenErrMaskCopy is the value actually used in subsequent expressions, computing directly into the non-volatile local (and only keeping volatile if it’s required for the toolchain workaround) would reduce noise and avoid an extra volatile write+read pair. For example, compute the mask into lenErrMaskCopy and use that consistently.

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

Comment thread wolfcrypt/src/rsa.c Outdated
Comment thread wolfcrypt/src/rsa.c Outdated
Comment thread wolfcrypt/src/rsa.c Outdated
@embhorn
Copy link
Copy Markdown
Member Author

embhorn commented Mar 24, 2026

Jenkins retest this please

@embhorn embhorn assigned wolfSSL-Bot and unassigned embhorn Mar 24, 2026
Copy link
Copy Markdown
Member

@dgarske dgarske left a comment

Choose a reason for hiding this comment

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

These changes feel "hacky". Perhaps they need "volatile" added? I liked the original code better. Can we have IAR ignore that specific warning?

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #10045

Scan targets checked: src, src-bugs, src-compliance, wolfcrypt-bugs, wolfcrypt-src

No new issues found in the changed files. ✅

@embhorn embhorn requested a review from dgarske March 25, 2026 12:48
@embhorn
Copy link
Copy Markdown
Member Author

embhorn commented Mar 25, 2026

These changes feel "hacky". Perhaps they need "volatile" added? I liked the original code better. Can we have IAR ignore that specific warning?

The issue is about multiple volatile statements on one line, and the warning is valid.
I changed the logic a bit and I think this new approach is better and more clear.

@embhorn embhorn assigned dgarske and unassigned embhorn Mar 25, 2026
@dgarske dgarske assigned wolfSSL-Bot and unassigned dgarske Mar 25, 2026
@dgarske dgarske requested a review from douzzer March 25, 2026 18:08
@douzzer douzzer merged commit 0a61997 into wolfSSL:master Mar 30, 2026
493 of 494 checks passed
@embhorn embhorn deleted the zd21385 branch April 24, 2026 21:33
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.

6 participants