Skip to content

Fix out-of-range integer brng reading past brng_list - #177

Open
vchamarthi wants to merge 1 commit into
IntelPython:masterfrom
vchamarthi:fix/validate-brng-token
Open

vchamarthi wants to merge 1 commit into
IntelPython:masterfrom
vchamarthi:fix/validate-brng-token

Conversation

@vchamarthi

Copy link
Copy Markdown
Contributor

Issue

brng_list has BRNG_KINDS = 11 entries, irk_brng_t defines 0–10. The integer
path of _parse_brng_token_ coerced the user value straight to irk_brng_t, which
only checks it fits the enum's underlying integer type. The seeding routines then
index brng_list[brng].

brng=11 returned uninitialized memory, no warning, no error:

mkl_random.MKLRandomState(1, brng=11).randint(0, 100, 6)
array([1744283392, 31525, 299244432, 22256, 1744285296, 31525])

brng=15 raised an oneMKL error; brng=-1 was accepted as a negative index.
Separately, if (brng): treated brng=0 as unset, but 0 is MT19937:

Intel oneMKL ERROR: Parameter 1 was incorrect on entry to vslGetStreamStateBrng
mkl_random.MKLRandomState(1, brng=0).randint(0, 100, 6)
array([0, 0, 0, 0, 0, 0]) # brng="MT19937" gives [13 56 84 80 76 6]

How found

Coverity Scan CID 653949, OVERRUN: "Overrunning callee's array of size 11 by
passing argument __pyx_v_brng_token (which evaluates to 15) in call to
irk_randomseed_mkl". Confirmed against randomkit.h:50, randomkit.cpp:111 and
the .pyx call path, then reproduced per-process on a build of master. The
brng=0 bug surfaced while writing the boundary test.

Distinct from the _seed_impl OUT_OF_BOUNDS finding already dismissed in
coverity/README.md, which is the tuple unpack and is a false positive.

Fix

  • Expose BRNG_KINDS from randomkit.h so the bound tracks the C table.
  • Range-check the integer path; out of range warns and falls back to MT19937,
    as an unrecognized generator name already does.
  • Use brng is not None instead of truthiness, so brng=0 is honoured.

Tests

5 out-of-range values assert warn plus fallback; 0 and 10 assert accepted,
silent, and equal to the same generator selected by name.

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.

1 participant