Skip to content

blob(windows): free CopyFileWindows and ReadFileUV through the task pointer, not under &mut self #37705

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 8 commits into
base: main
Choose a base branch
from
Open

blob(windows): free CopyFileWindows and ReadFileUV through the task pointer, not under &mut self #37705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7c20114
blob(windows): free CopyFileWindows / ReadFileUV through the task poi…
robobun Aug 12, 2026
c337d78
test: cover the copy-then-chmod chain and fd-backed reads on every pl…
robobun Aug 12, 2026
1074263
test: share the self-receiver-teardown lint with the sibling conversi…
robobun Aug 12, 2026
63b3f9e
blob(windows): tear the tasks down through Drop
robobun Aug 12, 2026
5446b77
blob(windows): shorten the new doc and SAFETY comments
robobun Aug 12, 2026
b18b9b8
test: drop the ArrayBufferSink allowlist entry, converted by #37716
robobun Aug 12, 2026
421d0f4
blob(windows): post the mkdirp hop through a cloned LoopHandle
robobun Aug 12, 2026
72707d3
blob(windows): open ReadFileUV's file with libuv's RDONLY
robobun Aug 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
153 changes: 31 additions & 122 deletions src/runtime/webcore/Blob.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6849,6 +6849,8 @@ bun_jsc::jsc_host_abi! {
// TODO: move to bun_sys?
/// Generic file-open helper used by ReadFile/WriteFile/CopyFile state machines,
/// modeled as a trait the target implements.
/// `get_fd` is POSIX-only: the Windows tasks (`ReadFileUV`, `WriteFileWindows`)
/// open through libuv themselves, since their completions may free them.
Comment thread
robobun marked this conversation as resolved.
pub trait FileOpener: Sized {
/// Override if you need different open flags; defaults to RDONLY.
const OPEN_FLAGS: i32 = bun_sys::O::RDONLY;
Expand All @@ -6872,18 +6874,8 @@ pub trait FileOpener: Sized {
) -> Retry {
Retry::No
}
#[cfg(windows)]
fn loop_(&self) -> *mut bun_libuv_sys::uv_loop_t;
#[cfg(windows)]
fn req(&mut self) -> &mut bun_libuv_sys::uv_fs_t;
/// Stash/retrieve the open completion callback across the libuv async hop.
/// Rust can't const-generic over fn
/// pointers, so the implementor stores it on `self` (e.g. next to `req`).
#[cfg(windows)]
fn set_open_callback(&mut self, cb: fn(&mut Self, Fd));
#[cfg(windows)]
fn open_callback(&self) -> fn(&mut Self, Fd);

#[cfg(not(windows))]
fn get_fd_by_opening(&mut self, callback: fn(&mut Self, Fd)) {
let mut buf = bun_paths::PathBuffer::uninit();
let path_string = match self.pathlike() {
Expand All @@ -6892,126 +6884,43 @@ pub trait FileOpener: Sized {
};
let path = path_string.slice_z(&mut buf);

#[cfg(windows)]
{
use bun_sys::ReturnCodeExt as _;
// Monomorphic libuv completion thunk — recovers `*mut Self` from
// `req.data`.
extern "C" fn wrapped_callback<S: FileOpener>(req: *mut bun_libuv_sys::uv_fs_t) {
use bun_sys::ReturnCodeExt as _;
// SAFETY: `req.data` was set to `self as *mut Self` below before
// `uv_fs_open` was queued; libuv guarantees `req` is valid here.
let self_: &mut S = unsafe { bun_ptr::callback_ctx::<S>((*req).data) };
{
// SAFETY: req points into self_.req(); cleanup before reuse.
scopeguard::defer! { unsafe { bun_libuv_sys::uv_fs_req_cleanup(req); } }
// SAFETY: req is the live uv_fs_t from the open request.
let result = unsafe { (*req).result };
if let Some(err_enum) = result.err_enum_e() {
let path_string_2 = match self_.pathlike() {
PathOrFileDescriptor::Path(p) => p.clone(),
PathOrFileDescriptor::Fd(_) => unreachable!(),
};
self_.set_errno(bun_errno::from_errno(err_enum as i32).into());
self_.set_system_error(
bun_sys::Error::from_code(err_enum, bun_sys::Tag::open)
.with_path(path_string_2.slice())
.to_system_error()
.into(),
);
self_.set_opened_fd(bun_sys::Fd::INVALID);
} else {
self_.set_opened_fd(Fd::from_uv(result.to_fd()));
}
loop {
match bun_sys::open(
path,
Self::OPEN_FLAGS | Self::OPENER_FLAGS,
crate::node::fs::DEFAULT_PERMISSION,
) {
bun_sys::Result::Ok(fd) => {
self.set_opened_fd(fd);
break;
}
let cb = self_.open_callback();
cb(self_, self_.opened_fd());
}

self.set_open_callback(callback);
let loop_ = self.loop_();
let self_ptr: *mut Self = core::ptr::from_mut(self);
// Derive `req` THROUGH `self_ptr` rather than via a fresh `self.req()`
// reborrow. Under Stacked Borrows, a direct `self.req()` here would
// create a sibling `&mut` that pops `self_ptr`'s tag, making the
// later deref in `wrapped_callback` (via `req.data`) UB. Going
// through the raw pointer keeps the reborrow as a child of
// `self_ptr`, so its provenance survives until the callback fires.
// SAFETY: `self_ptr` was just derived from a live `&mut self`.
let req = unsafe { (*self_ptr).req() };
// Stash `self` on the request BEFORE dispatch. libuv never touches
// `req.data`, so pre-setting is safe; doing it after `uv_fs_open`
// is a UAF when the call fails synchronously and `callback` frees
// `self` (ReadFileUV::on_finish → finalize → heap::take).
req.data = self_ptr.cast();
// SAFETY: loop_/req are live for the duration of the async open;
// req.data is consumed by `wrapped_callback::<Self>` above.
let rc = unsafe {
bun_libuv_sys::uv_fs_open(
loop_,
req,
path.as_ptr(),
Self::OPEN_FLAGS | Self::OPENER_FLAGS,
node::fs::DEFAULT_PERMISSION as i32,
Some(wrapped_callback::<Self>),
)
};
if let Some(errno) = rc.err_enum_e() {
self.set_errno(bun_errno::from_errno(errno as i32).into());
self.set_system_error(
bun_sys::Error::from_code(errno, bun_sys::Tag::open)
.with_path(path_string.slice())
.to_system_error()
.into(),
);
self.set_opened_fd(bun_sys::Fd::INVALID);
// `callback` may free `self` (see comment above) — must be the
// last thing we touch on this path.
callback(self, bun_sys::Fd::INVALID);
return;
}
return;
}

#[cfg(not(windows))]
{
loop {
match bun_sys::open(
path,
Self::OPEN_FLAGS | Self::OPENER_FLAGS,
crate::node::fs::DEFAULT_PERMISSION,
) {
bun_sys::Result::Ok(fd) => {
self.set_opened_fd(fd);
break;
}
bun_sys::Result::Err(err) => {
if err.get_errno() == bun_sys::E::ENOENT {
match self.try_mkdirp(err.clone(), path, path_string.slice()) {
Retry::Continue => continue,
Retry::Fail => {
// `mkdir_if_not_exists` already populated
// `errno`/`system_error` on the impl.
self.set_opened_fd(Fd::INVALID);
break;
}
Retry::No => {}
bun_sys::Result::Err(err) => {
if err.get_errno() == bun_sys::E::ENOENT {
match self.try_mkdirp(err.clone(), path, path_string.slice()) {
Retry::Continue => continue,
Retry::Fail => {
// `mkdir_if_not_exists` already populated
// `errno`/`system_error` on the impl.
Comment thread
robobun marked this conversation as resolved.
self.set_opened_fd(Fd::INVALID);
break;
}
Retry::No => {}
}
self.set_errno(bun_errno::from_errno(err.errno as i32).into());
self.set_system_error(jsc::SysErrorJsc::to_system_error(
&err.with_path(path_string.slice()),
));
self.set_opened_fd(Fd::INVALID);
break;
}
self.set_errno(bun_errno::from_errno(err.errno as i32).into());
self.set_system_error(jsc::SysErrorJsc::to_system_error(
&err.with_path(path_string.slice()),
));
self.set_opened_fd(Fd::INVALID);
break;
}
}

callback(self, self.opened_fd());
}

callback(self, self.opened_fd());
}

#[cfg(not(windows))]
fn get_fd(&mut self, callback: fn(&mut Self, Fd)) {
if self.opened_fd() != Fd::INVALID {
callback(self, self.opened_fd());
Expand Down
Loading