Skip to content

Commit ae3804f

Browse files
youknowoneyouknowone
authored
more hostenv isolation (#7886)
* Convert host_env Windows path/argv params from raw *const u16 to &WideCStr * Migrate remaining winapi raw u16 pointer signatures to typed references * Migrate winreg pub unsafe fn string parameters to typed references * Add ToPyException impls for host_env error types (PyPy wrap_oserror analog) * Add CheckLibcResult helper and apply to socket/fcntl/shm/posix_wasi * Add Win32 BOOL/HANDLE check helpers; apply check helpers across host_env * Apply Win32/libc check helpers to overlapped/testconsole/os.rs * Apply Win32 check helpers to winapi.rs (partial) * Apply Win32 check helpers across more winapi.rs functions * Apply Win32 check helpers to nt.rs (partial) * Add CheckWin32Sentinel helper; apply to nt.rs INVALID_HANDLE_VALUE/INVALID_FILE_ATTRIBUTES patterns * Add OwnedHandle / HandleToOwned helper; apply to mmap create_named_mapping leak path * Use OwnedHandle RAII in nt::pipe to eliminate manual cleanup on error path * Use OwnedHandle in nt::chmod_follow; hoist HandleToOwned import * Drop rustix dependency from vm crate Remove unused IntoPyException impl for rustix::io::Errno and the rustix entry in crates/vm/Cargo.toml. rustix is now only depended on by host_env. * Fix CI failures: cross-platform regressions - winapi.rs: pass None to create_event_w; the recent Option<&WideCStr> migration left one call site still passing a raw null pointer. - exceptions.rs: gate ToPyException for LockfError with cfg(any(unix, target_os = "wasi")), matching host_env::fcntl's own cfg. The previous cfg let it compile on wasm32-unknown-unknown where host_env::fcntl does not exist. - io_unsupported.rs: derive Eq on FileMode alongside PartialEq to satisfy clippy::derive_partial_eq_without_eq. * Fix CI failures: cfg gates and unused imports - exceptions.rs: gate ToPyException for LockfError with cfg(all(unix, not(target_os = "redox"))) to match the type's own cfg in host_env/src/fcntl.rs (LockfError is not built on wasi). - signal.rs: CheckLibcResult is only used in unix-gated functions; split import so it is not pulled in for windows. - mmap.rs: remove CheckWin32Handle from imports; no longer used after switching to HandleToOwned-based RAII. - overlapped.rs: remove INVALID_HANDLE_VALUE from connect_pipe import; the call now uses .check_valid(). * Fix CI failures: rustfmt and windows unused import - signal.rs: reorder cfg-gated imports per rustfmt. - socket.rs: gate ToPyException import to cfg(all(unix, not(target_os = "redox"))); it is only used inside sendmsg which has the same gate, so it was unused on windows. * Push remaining libc/extern callsites from vm into host_env Add host_env wrappers and replace the corresponding vm call sites: - host_env::errno::strerror_string for libc::strerror - host_env::io::write_stderr_raw for libc::write(STDERR_FILENO,...) - host_env::locale::localeconv_data reused from vm::format - host_env::os::abort for the inline abort extern - host_env::os::urandom wraps getrandom; getrandom moves from vm to host_env - host_env::posix::lchmod for the macOS/BSD lchmod extern - host_env::posix::fcopyfile for the macOS fcopyfile extern - host_env::nt::wputenv for the Windows _wputenv extern vm/format.rs's get_locale_info now uses host_env on both unix and windows instead of the unix-only libc::localeconv path. * Move time tz state and winsound FFI into host_env - host_env::time::tz: wraps the libc tzset/timezone/daylight/tzname globals on non-msvc, non-wasm32 targets. vm::stdlib::time now reads these via the typed wrappers instead of declaring its own externs. - host_env::winsound (windows): exposes PlaySoundW (via a typed PlaySoundSource enum), Beep, and MessageBeep. vm::stdlib::winsound drops its inline FFI block and routes through host_env. * Migrate unsetenv to host_env::nt::wputenv; rustfmt - vm::stdlib::os::unsetenv had a second _wputenv call site that still referenced the removed inline extern. Route it through host_env::nt::wputenv like putenv. - rustfmt fixups in exceptions.rs (boolean chain layout) and the two winsound files. * Address PR review comments - host_env::winapi::create_process: assert that the command_line buffer is NUL-terminated and that the env block ends with a double-NUL, matching the Win32 CreateProcessW contract. - stdlib::overlapped CreateEvent: replace WideCString::from_str_truncate with the fallible from_str(), so embedded NULs in the event name surface as ValueError instead of being silently truncated. - vm::exceptions::ReadlinkError::NotSymbolicLink now maps to OSError (matches Win32 ERROR_NOT_A_REPARSE_POINT semantics) rather than ValueError. - winreg::ConnectRegistry: route the non-zero return through the existing os_error_from_windows_code helper so the resulting exception carries the real winerror/message instead of a generic OSError. * Fix CI failures and address review follow-ups CI failures: - rustfmt cleanup in exceptions.rs after the ReadlinkError change. - vm/stdlib/os.rs: drop unused ToWideString import that the wputenv migration left behind. - vm/stdlib/winsound.rs: replace explicit `&*buf` with `&buf` to satisfy clippy::explicit_auto_deref. - Lib/test/test_format.py, Lib/test/test_types.py: drop the now-stale expectedFailureIfWindows decorators on the locale-format tests; the Windows path now reads real `localeconv` data via host_env so these tests pass. Review follow-ups: - host_env::winapi::create_process: switch the new buffer terminator checks from `assert!` to fallible validators returning `io::ErrorKind::InvalidInput`, so bad inputs stay recoverable at the API boundary. - host_env::winsound::play_sound: reject `Memory(_)` together with `SND_ASYNC` (lifetime-unsafe) and `SND_MEMORY` without a `Memory(_)` source. Expand `PlaySoundError` into a variant enum. - vm::stdlib::_winapi::CreateProcess: route the Win32 path/argv strings through `as_wtf8().to_wide_cstring()` like the rest of the Windows API surface; `expect_str()` could panic on Python strings containing lone surrogates.
1 parent d51069b commit ae3804f

45 files changed

Lines changed: 1170 additions & 1216 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎Cargo.lock‎

Lines changed: 1 addition & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Lib/test/test_format.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,6 @@ def test_non_ascii(self):
423423
self.assertEqual(format(1+2j, "\u2007^8"), "\u2007(1+2j)\u2007")
424424
self.assertEqual(format(0j, "\u2007^4"), "\u20070j\u2007")
425425

426-
@unittest.expectedFailureIfWindows("TODO: RUSTPYTHON; AssertionError: ',' not found in '123456789'")
427426
def test_locale(self):
428427
try:
429428
oldloc = locale.setlocale(locale.LC_ALL)

‎Lib/test/test_types.py‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,6 @@ def test(i, format_spec, result):
431431
test(123456, "1=20", '11111111111111123456')
432432
test(123456, "*=20", '**************123456')
433433

434-
@unittest.expectedFailureIfWindows("TODO: RUSTPYTHON; AssertionError: '1,234.57' != '1234.57'")
435434
@run_with_locale('LC_NUMERIC', 'en_US.UTF8', '')
436435
def test_float__format__locale(self):
437436
# test locale support for __format__ code 'n'
@@ -441,7 +440,6 @@ def test_float__format__locale(self):
441440
self.assertEqual(locale.format_string('%g', x, grouping=True), format(x, 'n'))
442441
self.assertEqual(locale.format_string('%.10g', x, grouping=True), format(x, '.10n'))
443442

444-
@unittest.expectedFailureIfWindows("TODO: RUSTPYTHON; AssertionError: '123,456,789,012,345,678,901,234,567,890' != '123456789012345678901234567890'")
445443
@run_with_locale('LC_NUMERIC', 'en_US.UTF8', '')
446444
def test_int__format__locale(self):
447445
# test locale support for __format__ code 'n' for integers

‎crates/host_env/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ license.workspace = true
1212
rustpython-wtf8 = { workspace = true }
1313

1414
bitflags = { workspace = true }
15+
getrandom = { workspace = true }
1516
libc = { workspace = true }
1617
num-traits = { workspace = true }
1718
parking_lot = { workspace = true }

‎crates/host_env/src/errno.rs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,18 @@
11
// spell-checker:disable
22

3+
/// Return the platform `strerror(errno)` message as an owned `String`.
4+
/// Returns `None` when the runtime gives no description for `errno`.
5+
#[cfg(any(unix, windows))]
6+
#[must_use]
7+
pub fn strerror_string(errno: i32) -> Option<String> {
8+
let ptr = unsafe { libc::strerror(errno) };
9+
if ptr.is_null() {
10+
return None;
11+
}
12+
let s = unsafe { core::ffi::CStr::from_ptr(ptr) }.to_string_lossy();
13+
Some(s.into_owned())
14+
}
15+
316
#[cfg(any(unix, windows, target_os = "wasi"))]
417
pub mod errors {
518
pub use libc::*;

‎crates/host_env/src/fcntl.rs‎

Lines changed: 10 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,14 @@ use std::io;
33
#[cfg(unix)]
44
use std::os::fd::BorrowedFd;
55

6+
use crate::os::CheckLibcResult;
7+
68
pub fn normalize_ioctl_request(request: i64) -> libc::c_ulong {
79
(request as u32) as libc::c_ulong
810
}
911

1012
pub fn fcntl_int(fd: i32, cmd: i32, arg: i32) -> io::Result<i32> {
11-
let ret = unsafe { libc::fcntl(fd, cmd, arg) };
12-
if ret < 0 {
13-
Err(io::Error::last_os_error())
14-
} else {
15-
Ok(ret)
16-
}
13+
unsafe { libc::fcntl(fd, cmd, arg) }.check_libc_neg()
1714
}
1815

1916
pub fn validate_fd(fd: i32) -> io::Result<()> {
@@ -56,12 +53,7 @@ pub fn set_blocking(fd: BorrowedFd<'_>, blocking: bool) -> io::Result<()> {
5653
}
5754

5855
pub fn fcntl_with_bytes(fd: i32, cmd: i32, arg: &mut [u8]) -> io::Result<i32> {
59-
let ret = unsafe { libc::fcntl(fd, cmd, arg.as_mut_ptr()) };
60-
if ret < 0 {
61-
Err(io::Error::last_os_error())
62-
} else {
63-
Ok(ret)
64-
}
56+
unsafe { libc::fcntl(fd, cmd, arg.as_mut_ptr()) }.check_libc_neg()
6557
}
6658

6759
/// # Safety
@@ -73,31 +65,16 @@ pub unsafe fn ioctl_ptr(
7365
request: libc::c_ulong,
7466
arg: *mut libc::c_void,
7567
) -> io::Result<i32> {
76-
let ret = unsafe { libc::ioctl(fd, request as _, arg) };
77-
if ret < 0 {
78-
Err(io::Error::last_os_error())
79-
} else {
80-
Ok(ret)
81-
}
68+
unsafe { libc::ioctl(fd, request as _, arg) }.check_libc_neg()
8269
}
8370

8471
pub fn ioctl_int(fd: i32, request: libc::c_ulong, arg: i32) -> io::Result<i32> {
85-
let ret = unsafe { libc::ioctl(fd, request as _, arg) };
86-
if ret < 0 {
87-
Err(io::Error::last_os_error())
88-
} else {
89-
Ok(ret)
90-
}
72+
unsafe { libc::ioctl(fd, request as _, arg) }.check_libc_neg()
9173
}
9274

9375
#[cfg(not(any(target_os = "wasi", target_os = "redox")))]
9476
pub fn flock(fd: i32, operation: i32) -> io::Result<i32> {
95-
let ret = unsafe { libc::flock(fd, operation) };
96-
if ret < 0 {
97-
Err(io::Error::last_os_error())
98-
} else {
99-
Ok(ret)
100-
}
77+
unsafe { libc::flock(fd, operation) }.check_libc_neg()
10178
}
10279

10380
#[cfg(not(any(target_os = "wasi", target_os = "redox")))]
@@ -137,7 +114,7 @@ pub fn lockf(fd: i32, cmd: i32, len: i64, start: i64, whence: i32) -> Result<i32
137114
..unsafe { core::mem::zeroed() }
138115
};
139116

140-
let ret = unsafe {
117+
unsafe {
141118
libc::fcntl(
142119
fd,
143120
if (cmd & libc::LOCK_NB) != 0 {
@@ -147,10 +124,7 @@ pub fn lockf(fd: i32, cmd: i32, len: i64, start: i64, whence: i32) -> Result<i32
147124
},
148125
&lock,
149126
)
150-
};
151-
if ret < 0 {
152-
Err(LockfError::Io(io::Error::last_os_error()))
153-
} else {
154-
Ok(ret)
155127
}
128+
.check_libc_neg()
129+
.map_err(LockfError::Io)
156130
}

‎crates/host_env/src/io.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,3 +252,13 @@ pub fn write_once(fd: crt_fd::Borrowed<'_>, buf: &[u8]) -> io::Result<usize> {
252252
pub fn close_owned_fd(fd: crt_fd::Owned) -> io::Result<()> {
253253
crt_fd::close(fd)
254254
}
255+
256+
/// Async-signal-safe raw write to the platform stderr file descriptor.
257+
/// Avoids `std::io::stderr()` locking so it is safe to call from fork
258+
/// children and signal handlers.
259+
#[cfg(unix)]
260+
pub fn write_stderr_raw(buf: &[u8]) {
261+
unsafe {
262+
let _ = libc::write(libc::STDERR_FILENO, buf.as_ptr().cast(), buf.len());
263+
}
264+
}

‎crates/host_env/src/io_unsupported.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ const O_TRUNC: i32 = 0x0400;
1717
const O_EXCL: i32 = 0x0800;
1818

1919
bitflags::bitflags! {
20-
#[derive(Copy, Clone, Debug, PartialEq)]
20+
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
2121
pub struct FileMode: u8 {
2222
const CREATED = 0b0001;
2323
const READABLE = 0b0010;

‎crates/host_env/src/lib.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,4 +83,6 @@ pub mod winapi;
8383
#[cfg(windows)]
8484
pub mod winreg;
8585
#[cfg(windows)]
86+
pub mod winsound;
87+
#[cfg(windows)]
8688
pub mod wmi;

‎crates/host_env/src/mmap.rs‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,14 @@
55

66
use std::io;
77

8+
#[cfg(windows)]
9+
use crate::windows::{CheckWin32Bool, HandleToOwned};
810
#[cfg(unix)]
911
use crate::{crt_fd, fileutils, posix};
1012
use memmap2::{Mmap, MmapMut, MmapOptions};
1113
#[cfg(windows)]
14+
use std::os::windows::io::{AsRawHandle, IntoRawHandle};
15+
#[cfg(windows)]
1216
use windows_sys::Win32::{
1317
Foundation::{
1418
CloseHandle, DUPLICATE_SAME_ACCESS, DuplicateHandle, GetLastError, HANDLE,
@@ -131,7 +135,7 @@ impl Drop for NamedMmap {
131135
#[cfg(windows)]
132136
pub fn duplicate_handle(handle: Handle) -> io::Result<Handle> {
133137
let mut new_handle: Handle = INVALID_HANDLE;
134-
let result = unsafe {
138+
unsafe {
135139
DuplicateHandle(
136140
GetCurrentProcess(),
137141
handle,
@@ -141,12 +145,9 @@ pub fn duplicate_handle(handle: Handle) -> io::Result<Handle> {
141145
0,
142146
DUPLICATE_SAME_ACCESS,
143147
)
144-
};
145-
if result == 0 {
146-
Err(io::Error::last_os_error())
147-
} else {
148-
Ok(new_handle)
149148
}
149+
.check_win32_bool()?;
150+
Ok(new_handle)
150151
}
151152

152153
#[cfg(windows)]
@@ -187,13 +188,9 @@ pub fn is_invalid_handle_value(handle: isize) -> bool {
187188

188189
#[cfg(windows)]
189190
pub fn extend_file(handle: Handle, size: i64) -> io::Result<()> {
190-
if unsafe { SetFilePointerEx(handle, size, core::ptr::null_mut(), FILE_BEGIN) } == 0 {
191-
return Err(io::Error::last_os_error());
192-
}
193-
if unsafe { SetEndOfFile(handle) } == 0 {
194-
return Err(io::Error::last_os_error());
195-
}
196-
Ok(())
191+
unsafe { SetFilePointerEx(handle, size, core::ptr::null_mut(), FILE_BEGIN) }
192+
.check_win32_bool()?;
193+
unsafe { SetEndOfFile(handle) }.check_win32_bool()
197194
}
198195

199196
#[cfg(unix)]
@@ -210,11 +207,7 @@ pub fn close_handle(handle: Handle) {
210207

211208
#[cfg(windows)]
212209
pub fn flush_view(ptr: *const core::ffi::c_void, size: usize) -> io::Result<()> {
213-
if unsafe { FlushViewOfFile(ptr, size) } == 0 {
214-
Err(io::Error::last_os_error())
215-
} else {
216-
Ok(())
217-
}
210+
unsafe { FlushViewOfFile(ptr, size) }.check_win32_bool()
218211
}
219212

220213
#[cfg(windows)]
@@ -252,21 +245,28 @@ pub fn create_named_mapping(
252245
size_lo,
253246
tag_wide.as_ptr(),
254247
)
255-
};
256-
if map_handle.is_null() {
257-
return Err(io::Error::last_os_error());
258248
}
249+
.into_owned()
250+
.ok_or_else(io::Error::last_os_error)?;
259251

260252
let off_hi = (offset as u64 >> 32) as u32;
261253
let off_lo = offset as u32;
262-
let view = unsafe { MapViewOfFile(map_handle, desired_access, off_hi, off_lo, map_size) };
254+
let view = unsafe {
255+
MapViewOfFile(
256+
map_handle.as_raw_handle() as Handle,
257+
desired_access,
258+
off_hi,
259+
off_lo,
260+
map_size,
261+
)
262+
};
263263
if view.Value.is_null() {
264-
unsafe { CloseHandle(map_handle) };
264+
// `map_handle` is closed automatically when dropped on this error path.
265265
return Err(io::Error::last_os_error());
266266
}
267267

268268
Ok(NamedMmap {
269-
map_handle,
269+
map_handle: map_handle.into_raw_handle() as Handle,
270270
view_ptr: view.Value as *mut u8,
271271
len: map_size,
272272
})

0 commit comments

Comments
 (0)