Skip to content

fix u16 truncation in slab random quarantine indexing#329

Open
thomasbuilds wants to merge 1 commit into
GrapheneOS:mainfrom
thomasbuilds:fix-slab-quarantine-random-u16-truncation
Open

fix u16 truncation in slab random quarantine indexing#329
thomasbuilds wants to merge 1 commit into
GrapheneOS:mainfrom
thomasbuilds:fix-slab-quarantine-random-u16-truncation

Conversation

@thomasbuilds
Copy link
Copy Markdown
Contributor

@thomasbuilds thomasbuilds commented May 10, 2026

deallocate_small() computes:

size_t slab_quarantine_random_length = SLAB_QUARANTINE_RANDOM_LENGTH << quarantine_shift;
size_t random_index = get_random_u16_uniform(&c->rng, slab_quarantine_random_length);

get_random_u16_uniform() takes a u16 bound, so slab_quarantine_random_length is silently truncated. With CONFIG_EXTENDED_SIZE_CLASSES=true (the default), quarantine_shift for the smallest size class is MAX_SLAB_SIZE_CLASS_SHIFT - MIN_SLAB_SIZE_CLASS_SHIFT = 13, so the shifted length is SLAB_QUARANTINE_RANDOM_LENGTH * 8192. Any SLAB_QUARANTINE_RANDOM_LENGTH >= 8 causes the shifted length to exceed the u16 range and truncate modulo 65536. Values that are multiples of 8 truncate to 0, causing get_random_u16_uniform to return 0 unconditionally and collapse the random quarantine to slot 0 for that size class. Non-multiples of 8 above the threshold truncate to a smaller nonzero bound, silently using only a prefix of the quarantine array. Without extended size classes, the threshold is SLAB_QUARANTINE_RANDOM_LENGTH >= 8 at quarantine_shift = 10, with collapse-to-zero for multiples of 64.

The static_assert at the top of h_malloc.c documents SLAB_QUARANTINE_RANDOM_LENGTH as valid up to 65536, but most of that range silently misbehaves at the call site.

The default configuration (SLAB_QUARANTINE_RANDOM_LENGTH=1) shifts to 8192, which fits in u16 and is unaffected. The corresponding queue path uses size_t arithmetic directly without a uniform-bound call, so it is not affected.

Fix: pick the random helper at compile time based on the worst-case bound. Use get_random_u16_uniform() when SLAB_QUARANTINE_RANDOM_LENGTH << (MAX_SLAB_SIZE_CLASS_SHIFT - MIN_SLAB_SIZE_CLASS_SHIFT) <= UINT16_MAX (which covers the default), get_random_u32_uniform() otherwise. The worst case is 65536 << 13 = 2^29, which fits in u32, so u64 is never required. Adds get_random_u32 and get_random_u32_uniform since they did not exist

@thomasbuilds thomasbuilds marked this pull request as ready for review May 10, 2026 06:18
@thestinger
Copy link
Copy Markdown
Member

This shouldn't use u64 when u32 is all that's required it also shouldn't use u32 unconditionally when usually only u16 is needed. Generating the random data takes time and it's better to use less of it which is why this is being done in the first place.

slab_quarantine_random_length = SLAB_QUARANTINE_RANDOM_LENGTH << quarantine_shift can exceed 65535, but was passed to get_random_u16_uniform() whose bound is u16. With CONFIG_EXTENDED_SIZE_CLASSES, the smallest size class has quarantine_shift=13, so any SLAB_QUARANTINE_RANDOM_LENGTH >= 8 truncates the bound modulo 65536.

Pick the random helper at compile time based on the worst-case bound: u16 when SLAB_QUARANTINE_RANDOM_LENGTH << (MAX_SLAB_SIZE_CLASS_SHIFT - MIN_SLAB_SIZE_CLASS_SHIFT) fits in u16 (the default), u32 otherwise. The worst case (65536 << 13 = 2^29) fits in u32, so u64 is never required.

Add get_random_u32 and get_random_u32_uniform helpers.
@thomasbuilds thomasbuilds force-pushed the fix-slab-quarantine-random-u16-truncation branch from c9656a6 to b017d7d Compare May 10, 2026 18:42
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.

2 participants