Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
46 changes: 42 additions & 4 deletions kernel/src/ipc/fd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,20 @@ impl Default for FdTable {

impl Clone for FdTable {
fn clone(&self) -> Self {
FdTable {
fds: alloc::boxed::Box::new((*self.fds).clone()),
let cloned_fds = alloc::boxed::Box::new((*self.fds).clone());

// Increment pipe reference counts for all cloned pipe fds
for fd_opt in cloned_fds.iter() {
if let Some(fd_entry) = fd_opt {
match &fd_entry.kind {
FdKind::PipeRead(buffer) => buffer.lock().add_reader(),
FdKind::PipeWrite(buffer) => buffer.lock().add_writer(),
_ => {}
}
}
}

FdTable { fds: cloned_fds }
}
}

Expand Down Expand Up @@ -179,8 +190,21 @@ impl FdTable {

let fd_entry = self.fds[old_fd as usize].clone().ok_or(9)?;

// Close new_fd if it's open (silently ignore errors)
let _ = self.close(new_fd);
// If new_fd is open, close it and decrement pipe ref counts
if let Some(old_entry) = self.fds[new_fd as usize].take() {
match old_entry.kind {
FdKind::PipeRead(buffer) => buffer.lock().close_read(),
FdKind::PipeWrite(buffer) => buffer.lock().close_write(),
_ => {}
}
}

// Increment pipe reference counts for the duplicated fd
match &fd_entry.kind {
FdKind::PipeRead(buffer) => buffer.lock().add_reader(),
FdKind::PipeWrite(buffer) => buffer.lock().add_writer(),
_ => {}
}

self.fds[new_fd as usize] = Some(fd_entry);
Ok(new_fd)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Race condition in dup2 when duplicating to same fd

When old_fd == new_fd, dup2 calls close_read()/close_write() and then add_reader()/add_writer() in separate lock operations. Between these calls, the pipe's reader/writer count can temporarily be zero. A concurrent write during this window will see readers == 0 and return EPIPE, even though the pipe isn't actually closed. Per POSIX, dup2(fd, fd) should return fd immediately without modifying anything. Adding an early return when old_fd == new_fd would prevent this race.

Fix in Cursor Fix in Web

Expand All @@ -196,13 +220,27 @@ impl FdTable {

let fd_entry = self.fds[old_fd as usize].clone().ok_or(9)?;

// Increment pipe reference counts for the duplicated fd
match &fd_entry.kind {
FdKind::PipeRead(buffer) => buffer.lock().add_reader(),
FdKind::PipeWrite(buffer) => buffer.lock().add_writer(),
_ => {}
}

// Find lowest available slot
for i in 0..MAX_FDS {
if self.fds[i].is_none() {
self.fds[i] = Some(fd_entry);
return Ok(i as i32);
}
}

// No slot found - need to decrement the counts we just added
match &fd_entry.kind {
FdKind::PipeRead(buffer) => buffer.lock().close_read(),
FdKind::PipeWrite(buffer) => buffer.lock().close_write(),
_ => {}
}
Err(24) // EMFILE
}
}
10 changes: 10 additions & 0 deletions kernel/src/ipc/pipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,16 @@ impl PipeBuffer {
}
}

/// Add a reader (used when duplicating pipe read fds)
pub fn add_reader(&mut self) {
self.readers += 1;
}

/// Add a writer (used when duplicating pipe write fds)
pub fn add_writer(&mut self) {
self.writers += 1;
}

/// Get the number of bytes available to read
#[allow(dead_code)]
pub fn available(&self) -> usize {
Expand Down
14 changes: 5 additions & 9 deletions kernel/src/syscall/pipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,15 +148,11 @@ pub fn sys_close(fd: i32) -> SyscallResult {
FdKind::StdIo(_) => {
log::debug!("sys_close: Closed stdio fd={}", fd);
}
FdKind::UdpSocket(socket_ref) => {
// Unbind the socket if it was bound
let socket = socket_ref.lock();
if let Some(port) = socket.local_port {
crate::socket::SOCKET_REGISTRY.unbind_udp(port);
log::debug!("sys_close: Closed UDP socket fd={}, unbound port {}", fd, port);
} else {
log::debug!("sys_close: Closed unbound UDP socket fd={}", fd);
}
FdKind::UdpSocket(_) => {
// Socket unbinding is handled by UdpSocket::Drop when the last
// Arc reference is released, allowing shared sockets (via dup/fork)
// to remain bound until all references are closed.
log::debug!("sys_close: Closed UDP socket fd={}", fd);
}
}
SyscallResult::Ok(0)
Expand Down
Loading