Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 82b692fc69fc58e128e35db1a2d0d5e5ae3bbec5
https://github.com/WebKit/WebKit/commit/82b692fc69fc58e128e35db1a2d0d5e5ae3bbec5
Author: Marcus Plutowski <[email protected]>
Date: 2026-07-01 (Wed, 01 Jul 2026)
Changed paths:
M Source/bmalloc/libpas/src/libpas/pas_race_test_hooks.h
M Source/bmalloc/libpas/src/libpas/pas_segregated_heap.c
M Source/bmalloc/libpas/src/libpas/pas_segregated_heap_inlines.h
M Source/bmalloc/libpas/src/test/RaceTests.cpp
Log Message:
-----------
[libpas] Fix race in pas_segregated_heap_medium_size_directory_for_index
https://bugs.webkit.org/show_bug.cgi?id=314829
rdar://175683224
Reviewed by Dan Hecht and Yijia Huang.
The medium-size directory is protected by both the heap-lock and a
seqlock (i.e. pas_mutation_count on rare_data->mutation_count).
The seqlock allows for readers to access the rare_data without
acquiring a lock; if the mutation-count shows that the object is being
mutated, then the reader is supposed to fall-back to acquiring the
heap-lock. However, as an optimization, if the heap-lock is already
held, the reader skips the seqlock step.
Previously, pas_segregated_heap_size_directory_for_index_slow would
always call pas_segregated_heap_medium_size_directory_for_index with
pas_lock_is_held. This meant that in certain cases, slow-path calls
will hit a race that results in a malloc call being given a slot within
segregated-heap slot that is actually too small for the requested size.
Operationally, what would happen is more or less
1. ThreadA tries allocating an object X and searches for a
matching medium-size-directory, but fails to find one.
2. This means it needs to create a new medium-size-directory.
This requires inserting a new entry into the medium-tuple array.
ThreadA determines that the new tuple should live at index M.
3. The medium-tuple array is sorted and packed, so inserting into the
middle requires `memmove`'ing all tuples down one slot --
so ThreadA does so.
At this point, slot[M] still points to the old entry,
and still has the old begin/end.
3. ThreadA then begins overwriting the slot[M], first
overwriting the directory-pointer.
At this point, slot[M] still has the old begin/end,
but points to the new directory.
4. ThreadB tries allocating an object Y. It should take the seqlock
here, but incorrectly assumes that it holds the heap-lock and thus
doesn't need to.
5. ThreadB searches for a matching medium-size-directory, and finds
slot[M] has a begin/end pair that matches its expectations.
6. ThreadB then reads slot[M]'s directory pointer, thus
getting a directory with a mismatched size.
7. ThreadB then asks that directory for an allocation-slot,
and gets one back: however, the size of this slot is
necessarily too small (because of the sorted-insertion
requirement in #3 above).
Therefore, the caller thinks that it got e.g. an 8192B
slot but in fact only got a 7162B slot. A write to the
latter bytes of the slot will corrupt the memory of the
subsequent allocation.
N.b. step #3 is also a race on its own, but it's harder to capture
since we can't hook in the middle of the memmove.
Best as I can tell, at some point this hardcoded heap-lock hold mode
was actually correct, since the primary path to get here does acquire
the lock. However, there's another path (perhaps added later?
unfortunately it's all lumped into the commit that added libpas to the
repo) that doesn't do so. So the correct fix is to pipe the heap-lock
hold status up far enough that we either get to a function that takes
the hold-mode as a parameter, or one which asserts that the heap-lock
is held.
* Source/bmalloc/libpas/src/libpas/test/RaceTests.cpp: added
new test to exhibit the race described above.
Originally-landed-as: 305413.966@safari-7624-branch (58ebed1e9cec).
rdar://180436276
Canonical link: https://commits.webkit.org/316275@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
