Skip to content

Commit 6ced254

Browse files
rvolosatovsrvolosatovs
authored
[24.0.4] backport fd_renumber fixes (#11278)
* fix(wasip1): prevent duplicate FD usage The implementation assumed that only the runtime could ever issue FDs, however that's not the case in p1, where guests can choose arbitrary FDs to use (e.g. via `fd_renumber`). Due to incorrect accounting, guests could "mark" arbitrary FDs as "free" and trigger a panic in the host by requesting a new FD. Signed-off-by: Roman Volosatovs <[email protected]> * test(wasip1): expand `fd_renumber` test Signed-off-by: Roman Volosatovs <[email protected]> * refactor(wasip1): do not modify descriptors on `fd_renumber(n, n)` Since `remove` is now only used once, remove it. As a sideffect, this makes the implementation more explicit . Signed-off-by: Roman Volosatovs <[email protected]> * fix(wasip1-adapter): prevent `unreachable` panic on `fd_renumber` Signed-off-by: Roman Volosatovs <[email protected]> * doc: add release notes Signed-off-by: Roman Volosatovs <[email protected]> * doc: reference the CVE prtest:full Signed-off-by: Roman Volosatovs <[email protected]> * doc: add PR reference prtest:full Signed-off-by: Roman Volosatovs <[email protected]> * chore: run `cargo fmt --all` Signed-off-by: Roman Volosatovs <[email protected]> --------- Signed-off-by: Roman Volosatovs <[email protected]>
1 parent 6f07581 commit 6ced254

File tree

4 files changed

+99
-55
lines changed

4 files changed

+99
-55
lines changed

‎RELEASES.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,15 @@
1+
## 24.0.4
2+
3+
Released 2025-07-18.
4+
5+
### Fixed
6+
7+
* Fix a panic in the host caused by preview1 guests using `fd_renumber`.
8+
[CVE-2025-53901](https://github.com/bytecodealliance/wasmtime/security/advisories/GHSA-fm79-3f68-h2fc).
9+
10+
* Fix a panic in the preview1 adapter caused by guests using `fd_renumber`.
11+
[#11277](https://github.com/bytecodealliance/wasmtime/pull/11277)
12+
113
## 24.0.3
214

315
Released 2025-06-24.

‎crates/test-programs/src/bin/preview1_renumber.rs‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,46 @@ unsafe fn test_renumber(dir_fd: wasi::Fd) {
7272
);
7373

7474
wasi::fd_close(fd_to).expect("closing a file");
75+
76+
wasi::fd_renumber(0, 0).expect("renumbering 0 to 0");
77+
let fd_file3 = wasi::path_open(
78+
dir_fd,
79+
0,
80+
"file3",
81+
wasi::OFLAGS_CREAT,
82+
wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_WRITE,
83+
0,
84+
0,
85+
)
86+
.expect("opening a file");
87+
assert!(
88+
fd_file3 > libc::STDERR_FILENO as wasi::Fd,
89+
"file descriptor range check",
90+
);
91+
92+
wasi::fd_renumber(fd_file3, 127).expect("renumbering FD to 127");
93+
match wasi::fd_renumber(127, u32::MAX) {
94+
Err(wasi::ERRNO_NOMEM) => {
95+
// The preview1 adapter cannot handle more than 128 descriptors
96+
eprintln!("fd_renumber({fd_file3}, {}) returned NOMEM", u32::MAX)
97+
}
98+
res => res.expect("renumbering FD to `u32::MAX`"),
99+
}
100+
101+
let fd_file4 = wasi::path_open(
102+
dir_fd,
103+
0,
104+
"file4",
105+
wasi::OFLAGS_CREAT,
106+
wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_WRITE,
107+
0,
108+
0,
109+
)
110+
.expect("opening a file");
111+
assert!(
112+
fd_file4 > libc::STDERR_FILENO as wasi::Fd,
113+
"file descriptor range check",
114+
);
75115
}
76116

77117
fn main() {

‎crates/wasi-preview1-component-adapter/src/descriptors.rs‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -326,8 +326,8 @@ impl Descriptors {
326326
.ok_or(wasi::ERRNO_BADF)
327327
}
328328

329-
// Internal: close a fd, returning the descriptor.
330-
fn close_(&mut self, fd: Fd) -> Result<Descriptor, Errno> {
329+
// Close an fd.
330+
pub fn close(&mut self, fd: Fd) -> Result<(), Errno> {
331331
// Throw an error if closing an fd which is already closed
332332
match self.get(fd)? {
333333
Descriptor::Closed(_) => Err(wasi::ERRNO_BADF)?,
@@ -337,12 +337,7 @@ impl Descriptors {
337337
let last_closed = self.closed;
338338
let prev = std::mem::replace(self.get_mut(fd)?, Descriptor::Closed(last_closed));
339339
self.closed = Some(fd);
340-
Ok(prev)
341-
}
342-
343-
// Close an fd.
344-
pub fn close(&mut self, fd: Fd) -> Result<(), Errno> {
345-
drop(self.close_(fd)?);
340+
drop(prev);
346341
Ok(())
347342
}
348343

@@ -362,11 +357,20 @@ impl Descriptors {
362357
while self.table_len.get() as u32 <= to_fd {
363358
self.push_closed()?;
364359
}
365-
// Then, close from_fd and put its contents into to_fd:
366-
let desc = self.close_(from_fd)?;
367-
// TODO FIXME if this overwrites a preopen, do we need to clear it from the preopen table?
368-
*self.get_mut(to_fd)? = desc;
369-
360+
// Throw an error if renumbering a closed fd
361+
match self.get(from_fd)? {
362+
Descriptor::Closed(_) => Err(wasi::ERRNO_BADF)?,
363+
_ => {}
364+
}
365+
// Close from_fd and put its contents into to_fd
366+
if from_fd != to_fd {
367+
// Mutate the descriptor to be closed, and push the closed fd onto the head of the linked list:
368+
let last_closed = self.closed;
369+
let desc = std::mem::replace(self.get_mut(from_fd)?, Descriptor::Closed(last_closed));
370+
self.closed = Some(from_fd);
371+
// TODO FIXME if this overwrites a preopen, do we need to clear it from the preopen table?
372+
*self.get_mut(to_fd)? = desc;
373+
}
370374
Ok(())
371375
}
372376

‎crates/wasi/src/preview1.rs‎

Lines changed: 30 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,8 @@ use crate::{
7676
FsError, IsATTY, ResourceTable, StreamError, StreamResult, WasiCtx, WasiImpl, WasiView,
7777
};
7878
use anyhow::{bail, Context};
79-
use std::collections::{BTreeMap, HashSet};
79+
use std::collections::{btree_map, BTreeMap, BTreeSet, HashSet};
8080
use std::mem::{self, size_of, size_of_val};
81-
use std::ops::{Deref, DerefMut};
8281
use std::slice;
8382
use std::sync::atomic::{AtomicU64, Ordering};
8483
use std::sync::Arc;
@@ -318,21 +317,7 @@ struct WasiPreview1Adapter {
318317
#[derive(Debug, Default)]
319318
struct Descriptors {
320319
used: BTreeMap<u32, Descriptor>,
321-
free: Vec<u32>,
322-
}
323-
324-
impl Deref for Descriptors {
325-
type Target = BTreeMap<u32, Descriptor>;
326-
327-
fn deref(&self) -> &Self::Target {
328-
&self.used
329-
}
330-
}
331-
332-
impl DerefMut for Descriptors {
333-
fn deref_mut(&mut self) -> &mut Self::Target {
334-
&mut self.used
335-
}
320+
free: BTreeSet<u32>,
336321
}
337322

338323
impl Descriptors {
@@ -409,42 +394,34 @@ impl Descriptors {
409394

410395
/// Returns next descriptor number, which was never assigned
411396
fn unused(&self) -> Result<u32> {
412-
match self.last_key_value() {
397+
match self.used.last_key_value() {
413398
Some((fd, _)) => {
414399
if let Some(fd) = fd.checked_add(1) {
415400
return Ok(fd);
416401
}
417-
if self.len() == u32::MAX as usize {
402+
if self.used.len() == u32::MAX as usize {
418403
return Err(types::Errno::Loop.into());
419404
}
420405
// TODO: Optimize
421406
Ok((0..u32::MAX)
422407
.rev()
423-
.find(|fd| !self.contains_key(fd))
408+
.find(|fd| !self.used.contains_key(fd))
424409
.expect("failed to find an unused file descriptor"))
425410
}
426411
None => Ok(0),
427412
}
428413
}
429414

430-
/// Removes the [Descriptor] corresponding to `fd`
431-
fn remove(&mut self, fd: types::Fd) -> Option<Descriptor> {
432-
let fd = fd.into();
433-
let desc = self.used.remove(&fd)?;
434-
self.free.push(fd);
435-
Some(desc)
436-
}
437-
438415
/// Pushes the [Descriptor] returning corresponding number.
439416
/// This operation will try to reuse numbers previously removed via [`Self::remove`]
440417
/// and rely on [`Self::unused`] if no free numbers are recorded
441418
fn push(&mut self, desc: Descriptor) -> Result<u32> {
442-
let fd = if let Some(fd) = self.free.pop() {
419+
let fd = if let Some(fd) = self.free.pop_last() {
443420
fd
444421
} else {
445422
self.unused()?
446423
};
447-
assert!(self.insert(fd, desc).is_none());
424+
assert!(self.used.insert(fd, desc).is_none());
448425
Ok(fd)
449426
}
450427
}
@@ -485,15 +462,15 @@ impl Transaction<'_> {
485462
/// Returns [`types::Errno::Badf`] if no [`Descriptor`] is found
486463
fn get_descriptor(&self, fd: types::Fd) -> Result<&Descriptor> {
487464
let fd = fd.into();
488-
let desc = self.descriptors.get(&fd).ok_or(types::Errno::Badf)?;
465+
let desc = self.descriptors.used.get(&fd).ok_or(types::Errno::Badf)?;
489466
Ok(desc)
490467
}
491468

492469
/// Borrows [`File`] corresponding to `fd`
493470
/// if it describes a [`Descriptor::File`]
494471
fn get_file(&self, fd: types::Fd) -> Result<&File> {
495472
let fd = fd.into();
496-
match self.descriptors.get(&fd) {
473+
match self.descriptors.used.get(&fd) {
497474
Some(Descriptor::File(file)) => Ok(file),
498475
_ => Err(types::Errno::Badf.into()),
499476
}
@@ -503,7 +480,7 @@ impl Transaction<'_> {
503480
/// if it describes a [`Descriptor::File`]
504481
fn get_file_mut(&mut self, fd: types::Fd) -> Result<&mut File> {
505482
let fd = fd.into();
506-
match self.descriptors.get_mut(&fd) {
483+
match self.descriptors.used.get_mut(&fd) {
507484
Some(Descriptor::File(file)) => Ok(file),
508485
_ => Err(types::Errno::Badf.into()),
509486
}
@@ -517,7 +494,7 @@ impl Transaction<'_> {
517494
/// Returns [`types::Errno::Spipe`] if the descriptor corresponds to stdio
518495
fn get_seekable(&self, fd: types::Fd) -> Result<&File> {
519496
let fd = fd.into();
520-
match self.descriptors.get(&fd) {
497+
match self.descriptors.used.get(&fd) {
521498
Some(Descriptor::File(file)) => Ok(file),
522499
Some(
523500
Descriptor::Stdin { .. } | Descriptor::Stdout { .. } | Descriptor::Stderr { .. },
@@ -550,7 +527,7 @@ impl Transaction<'_> {
550527
/// if it describes a [`Descriptor::Directory`]
551528
fn get_dir_fd(&self, fd: types::Fd) -> Result<Resource<filesystem::Descriptor>> {
552529
let fd = fd.into();
553-
match self.descriptors.get(&fd) {
530+
match self.descriptors.used.get(&fd) {
554531
Some(Descriptor::Directory { fd, .. }) => Ok(fd.borrowed()),
555532
_ => Err(types::Errno::Badf.into()),
556533
}
@@ -1360,11 +1337,13 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiP1Ctx {
13601337
_memory: &mut GuestMemory<'_>,
13611338
fd: types::Fd,
13621339
) -> Result<(), types::Error> {
1363-
let desc = self
1364-
.transact()?
1365-
.descriptors
1366-
.remove(fd)
1367-
.ok_or(types::Errno::Badf)?;
1340+
let desc = {
1341+
let fd = fd.into();
1342+
let mut st = self.transact()?;
1343+
let desc = st.descriptors.used.remove(&fd).ok_or(types::Errno::Badf)?;
1344+
st.descriptors.free.insert(fd);
1345+
desc
1346+
};
13681347
match desc {
13691348
Descriptor::Stdin { stream, .. } => {
13701349
streams::HostInputStream::drop(&mut self.as_wasi_impl(), stream)
@@ -1900,8 +1879,17 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiP1Ctx {
19001879
to: types::Fd,
19011880
) -> Result<(), types::Error> {
19021881
let mut st = self.transact()?;
1903-
let desc = st.descriptors.remove(from).ok_or(types::Errno::Badf)?;
1904-
st.descriptors.insert(to.into(), desc);
1882+
let from = from.into();
1883+
let btree_map::Entry::Occupied(desc) = st.descriptors.used.entry(from) else {
1884+
return Err(types::Errno::Badf.into());
1885+
};
1886+
let to = to.into();
1887+
if from != to {
1888+
let desc = desc.remove();
1889+
st.descriptors.free.insert(from);
1890+
st.descriptors.free.remove(&to);
1891+
st.descriptors.used.insert(to, desc);
1892+
}
19051893
Ok(())
19061894
}
19071895

0 commit comments

Comments
 (0)