Repository navigation
fs.readdir: options_with_file_types dereferences an unrooted object pointer across an allocation #7274
Description
crates/perry-runtime/src/fs/dirent.rs's options_with_file_types decodes a raw ObjectHeader pointer out of a NaN-boxed f64, then allocates, then dereferences the pointer it computed before the allocation:
pub(crate) unsafe fn options_with_file_types(options_value: f64) -> bool {
let bits = options_value.to_bits();
let value = crate::value::JSValue::from_bits(bits);
let raw_ptr = /* decode from options_value */;
if crate::value::addr_class::is_handle_band(raw_ptr) {
return false;
}
let obj_ptr = raw_ptr as *const crate::object::ObjectHeader;
let key = crate::string::js_string_from_bytes(b"withFileTypes".as_ptr(), 13); // <-- ALLOCATES
let val = crate::object::js_object_get_field_by_name(obj_ptr, key); // <-- stale obj_ptr
crate::value::js_is_truthy(f64::from_bits(val.bits())) != 0
}js_string_from_bytes can trigger a collection. If that cycle evacuates the options object, obj_ptr names a from-space address that has already been swept, and js_object_get_field_by_name reads an unrelated live object — or garbage. options_value is a plain Rust f64 local, not a GC root, so it is stale by the same argument; re-deriving the pointer after the allocation would not help on its own.
The file already knows how to do this
options_field_value, 30 lines below in the same file, has the same signature and handles it correctly:
pub(crate) unsafe fn options_field_value(
options_value: f64,
field: &[u8],
) -> Option<crate::value::JSValue> {
let scope = crate::gc::RuntimeHandleScope::new();
let options_handle = scope.root_nanbox_f64(options_value);
let bits = options_handle.get_nanbox_f64().to_bits();
...and it re-reads through the handle after its own allocating js_string_from_bytes call:
let key = crate::string::js_string_from_bytes(field.as_ptr(), field.len() as u32);
let refreshed_bits = options_handle.get_nanbox_f64().to_bits();
let refreshed_value = crate::value::JSValue::from_bits(refreshed_bits);
let refreshed_ptr = /* re-decode */;So the fix is self-contained — no signature change, no caller churn: give options_with_file_types a RuntimeHandleScope, root options_value, and re-read the handle after the js_string_from_bytes call, exactly as its sibling does.
Reachability
One caller, dirent.rs:382, on the fs.readdir / fs.readdirSync path:
let with_file_types = options_with_file_types(options_value);{ withFileTypes: true } is an object literal freshly allocated by the caller, so it is a young-generation object — precisely the kind a minor collection triggered by the very next allocation is most likely to move.
Notes
- Found by CodeRabbit while reviewing fix(lint): clear the GC store-site and address-classification gates (#7258, #7259) #7273, which touches the two lines above this hazard (converting the
< 0x1000handle floor toaddr_class::is_handle_band). It is pre-existing and unrelated to that change, so it was not folded in: fix(lint): clear the GC store-site and address-classification gates (#7258, #7259) #7273 is a lint-gate fix and a GC-rooting change belongs on its own reviewable/revertable commit. - Same family as the recent argument-rooting work: GC: #7207's spread-accessor reproducer still SIGSEGVs on main under the evac_minor arm (allocation-point relocation, not the safepoint route) #7217, fix(gc): root every argument of a cross-module direct call (#7240, by @jdalton) #7252, fix(gc): root rest-args and same-module call arguments; teach the checker about string-literal handles (#7154) #7270/fix(gc): root rest-args and same-module call arguments (#7270, by @jdalton) #7271.
- No reproducer yet — this is a read of the code, not an observed crash. It would present as a nondeterministic wrong
withFileTypesresult or a segfault under GC pressure, and evacuation makes it far more likely than the non-moving default did.
Activity
fix(gc): root fs.readdir's options object across the withFileTypes ke…
fix(gc): root fs.readdir's options object across the withFileTypes ke…
fix(gc): root fs.readdir's options object across the withFileTypes ke…
gc: enumerate the runtime-side GC-pointer holders, with a gate (#7231)
gc: enumerate the runtime-side GC-pointer holders, with a gate (#7231) (