Skip to content

BUG: fix CPU feature env diagnostic buffer overruns - #31905

Merged
ngoldbaum merged 3 commits into
numpy:mainfrom
stratakis:buffers_more_buffers
Jul 10, 2026
Merged

BUG: fix CPU feature env diagnostic buffer overruns#31905
ngoldbaum merged 3 commits into
numpy:mainfrom
stratakis:buffers_more_buffers

Conversation

@stratakis

Copy link
Copy Markdown
Contributor

PR summary

This is a follow up to gh-30877.

I've had on the back on my mind to verify and check some other issues I saw in the file and finally managed to get to it.

The original PR fixed a buffer overrun in CPU baseline validation. This PR fixes two similar issues in npy__cpu_check_env.

The two issues fixed:

  • nonexistent features: an accepted maximum-length environment variable value could write the terminating NUL one byte past the nexist stack buffer
  • unsupported dispatched features: repeated unsupported feature names could overflow the notsupp diagnostic buffer, which was sized from NPY_WITH_CPU_DISPATCH rather than from the accepted environment variable length.

Both fixes avoid the trailing-space-and-trim pattern when building the diagnostic strings.

Noting though that the fix for the notsupp buffer increases stack usage in npy__cpu_check_env because the buffer is now bounded by NPY__MAX_VAR_LEN instead of sizeof(NPY_WITH_CPU_DISPATCH) + 1. On my x86_64 laptop, -fstack-usage reports the function growing from 2224 bytes to 3184 bytes, a 960 bytes increase. The function is non-recursive and only runs while parsing CPU feature environment variables during import, however if the size icrease is deemed unacceptable I can change the approach to something like heap allocation, it will require more code changes though.

These tests are mainly sanitizer regressions, in non-sanitized builds, the previous out-of-bounds writes might not crash.

AI Disclosure

I used codex with the gpt 5.5 model. Guided to create the tests, (hence the tests are AI generated) and to review the C code changes, also to verify any incompatibilities (where it verified the stack increase) and to create small reproducers with the same logic for me to actually verify the issues exist.

Avoid the trailing-space-and-trim pattern for nonexistent CPU features in
npy__cpu_check_env. A maximum-length invalid feature name could write
the terminating NUL one byte past the end of `nexist`.
@ngoldbaum ngoldbaum added 00 - Bug 09 - Backport-Candidate PRs tagged should be backported labels Jul 9, 2026

@ngoldbaum ngoldbaum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just two very minor nits, otherwise this looks good to me. The increased stack size doesn't matter I think.

I'll probably merge this soon but let's ping @seiko2plus since he originally wrote this code.

Comment thread numpy/_core/tests/test_cpu_features.py Outdated
Comment thread numpy/_core/src/common/npy_cpu_features.c Outdated
stratakis added 2 commits July 9, 2026 21:47
Use the maximum accepted environment variable length for the `notsupp`
diagnostic buffer in npy__cpu_check_env as repeated unsupported features
could overflow the buffer.
Use %zu when formatting var_len, which is a size_t.
@stratakis
stratakis force-pushed the buffers_more_buffers branch from 716118d to 6a52575 Compare July 9, 2026 19:50
@stratakis

Copy link
Copy Markdown
Contributor Author

Rebased to address the comments.

@ngoldbaum

Copy link
Copy Markdown
Member

Thanks @stratakis!

@ngoldbaum
ngoldbaum merged commit 66230cc into numpy:main Jul 10, 2026
87 checks passed
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Jul 10, 2026
charris added a commit that referenced this pull request Jul 10, 2026
BUG: fix CPU feature env diagnostic buffer overruns (#31905)
@stratakis
stratakis deleted the buffers_more_buffers branch July 11, 2026 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants