From 3de5c42afd6ec87d06a4c959d7ee534e781ba61f Mon Sep 17 00:00:00 2001 From: LI Qing Date: Wed, 13 Dec 2023 11:29:03 +0800 Subject: [PATCH] Support the close-on-exec file descriptor flag --- kernel/aster-nix/src/device/pty/pty.rs | 4 +- kernel/aster-nix/src/fs/file_table.rs | 78 +++++++++++++++++----- kernel/aster-nix/src/syscall/accept.rs | 4 +- kernel/aster-nix/src/syscall/dup.rs | 12 +++- kernel/aster-nix/src/syscall/epoll.rs | 10 +-- kernel/aster-nix/src/syscall/execve.rs | 6 ++ kernel/aster-nix/src/syscall/fcntl.rs | 39 ++++++++--- kernel/aster-nix/src/syscall/open.rs | 13 +++- kernel/aster-nix/src/syscall/pipe.rs | 13 ++-- kernel/aster-nix/src/syscall/socket.rs | 9 ++- kernel/aster-nix/src/syscall/socketpair.rs | 13 ++-- 11 files changed, 153 insertions(+), 48 deletions(-) diff --git a/kernel/aster-nix/src/device/pty/pty.rs b/kernel/aster-nix/src/device/pty/pty.rs index 0c15af13b..32316ce6b 100644 --- a/kernel/aster-nix/src/device/pty/pty.rs +++ b/kernel/aster-nix/src/device/pty/pty.rs @@ -10,6 +10,7 @@ use crate::{ fs::{ device::{Device, DeviceId, DeviceType}, devpts::DevPts, + file_table::FdFlags, fs_resolver::FsPath, inode_handle::FileIo, utils::{AccessMode, Inode, InodeMode, IoctlCmd}, @@ -189,7 +190,8 @@ impl FileIo for PtyMaster { let fd = { let mut file_table = current.file_table().lock(); - file_table.insert(slave) + // TODO: deal with the O_CLOEXEC flag + file_table.insert(slave, FdFlags::empty()) }; Ok(fd) } diff --git a/kernel/aster-nix/src/fs/file_table.rs b/kernel/aster-nix/src/fs/file_table.rs index 1e1d57ff2..1304f6f52 100644 --- a/kernel/aster-nix/src/fs/file_table.rs +++ b/kernel/aster-nix/src/fs/file_table.rs @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MPL-2.0 -use core::cell::Cell; +use core::sync::atomic::{AtomicU8, Ordering}; use aster_util::slot_vec::SlotVec; @@ -49,20 +49,26 @@ impl FileTable { let mode = InodeMode::S_IWUSR; fs_resolver.open(&tty_path, flags, mode.bits()).unwrap() }; - table.put(FileTableEntry::new(Arc::new(stdin), false)); - table.put(FileTableEntry::new(Arc::new(stdout), false)); - table.put(FileTableEntry::new(Arc::new(stderr), false)); + table.put(FileTableEntry::new(Arc::new(stdin), FdFlags::empty())); + table.put(FileTableEntry::new(Arc::new(stdout), FdFlags::empty())); + table.put(FileTableEntry::new(Arc::new(stderr), FdFlags::empty())); Self { table, subject: Subject::new(), } } - pub fn dup(&mut self, fd: FileDescripter, new_fd: FileDescripter) -> Result { - let entry = self.table.get(fd as usize).map_or_else( - || return_errno_with_message!(Errno::ENOENT, "No such file"), - |e| Ok(e.clone()), - )?; + pub fn dup( + &mut self, + fd: FileDescripter, + new_fd: FileDescripter, + flags: FdFlags, + ) -> Result { + let file = self + .table + .get(fd as usize) + .map(|entry| entry.file.clone()) + .ok_or(Error::with_message(Errno::ENOENT, "No such file"))?; // Get the lowest-numbered available fd equal to or greater than `new_fd`. let get_min_free_fd = || -> usize { @@ -80,12 +86,13 @@ impl FileTable { }; let min_free_fd = get_min_free_fd(); + let entry = FileTableEntry::new(file, flags); self.table.put_at(min_free_fd, entry); Ok(min_free_fd as FileDescripter) } - pub fn insert(&mut self, item: Arc) -> FileDescripter { - let entry = FileTableEntry::new(item, false); + pub fn insert(&mut self, item: Arc, flags: FdFlags) -> FileDescripter { + let entry = FileTableEntry::new(item, flags); self.table.put(entry) as FileDescripter } @@ -93,8 +100,9 @@ impl FileTable { &mut self, fd: FileDescripter, item: Arc, + flags: FdFlags, ) -> Option> { - let entry = FileTableEntry::new(item, false); + let entry = FileTableEntry::new(item, FdFlags::empty()); let entry = self.table.put_at(fd as usize, entry); if entry.is_some() { let events = FdEvents::Close(fd); @@ -131,6 +139,29 @@ impl FileTable { closed_files } + pub fn close_files_on_exec(&mut self) -> Vec> { + let mut closed_files = Vec::new(); + let closed_fds: Vec = self + .table + .idxes_and_items() + .filter_map(|(idx, entry)| { + if entry.flags().contains(FdFlags::CLOEXEC) { + Some(idx as FileDescripter) + } else { + None + } + }) + .collect(); + for fd in closed_fds { + let entry = self.table.remove(fd as usize).unwrap(); + let events = FdEvents::Close(fd); + self.notify_fd_events(&events); + entry.notify_fd_events(&events); + closed_files.push(entry.file); + } + closed_files + } + pub fn get_file(&self, fd: FileDescripter) -> Result<&Arc> { self.table .get(fd as usize) @@ -196,15 +227,15 @@ impl Events for FdEvents {} pub struct FileTableEntry { file: Arc, - close_on_exec: Cell, + flags: AtomicU8, subject: Subject, } impl FileTableEntry { - pub fn new(file: Arc, close_on_exec: bool) -> Self { + pub fn new(file: Arc, flags: FdFlags) -> Self { Self { file, - close_on_exec: Cell::new(close_on_exec), + flags: AtomicU8::new(flags.bits()), subject: Subject::new(), } } @@ -213,6 +244,14 @@ impl FileTableEntry { &self.file } + pub fn flags(&self) -> FdFlags { + FdFlags::from_bits(self.flags.load(Ordering::Relaxed)).unwrap() + } + + pub fn set_flags(&self, flags: FdFlags) { + self.flags.store(flags.bits(), Ordering::Relaxed); + } + pub fn register_observer(&self, epoll: Weak>) { self.subject.register_observer(epoll, ()); } @@ -230,8 +269,15 @@ impl Clone for FileTableEntry { fn clone(&self) -> Self { Self { file: self.file.clone(), - close_on_exec: self.close_on_exec.clone(), + flags: AtomicU8::new(self.flags.load(Ordering::Relaxed)), subject: Subject::new(), } } } + +bitflags! { + pub struct FdFlags: u8 { + /// Close on exec + const CLOEXEC = 1; + } +} diff --git a/kernel/aster-nix/src/syscall/accept.rs b/kernel/aster-nix/src/syscall/accept.rs index b397a99c1..17f8a3fb6 100644 --- a/kernel/aster-nix/src/syscall/accept.rs +++ b/kernel/aster-nix/src/syscall/accept.rs @@ -2,7 +2,7 @@ use super::{SyscallReturn, SYS_ACCEPT}; use crate::{ - fs::file_table::FileDescripter, + fs::file_table::{FdFlags, FileDescripter}, log_syscall_entry, prelude::*, util::net::{get_socket_from_fd, write_socket_addr_to_user}, @@ -24,7 +24,7 @@ pub fn sys_accept( let connected_fd = { let current = current!(); let mut file_table = current.file_table().lock(); - file_table.insert(connected_socket) + file_table.insert(connected_socket, FdFlags::empty()) }; Ok(SyscallReturn::Return(connected_fd as _)) } diff --git a/kernel/aster-nix/src/syscall/dup.rs b/kernel/aster-nix/src/syscall/dup.rs index 3172d5564..cfb53a88b 100644 --- a/kernel/aster-nix/src/syscall/dup.rs +++ b/kernel/aster-nix/src/syscall/dup.rs @@ -1,7 +1,11 @@ // SPDX-License-Identifier: MPL-2.0 use super::{SyscallReturn, SYS_DUP, SYS_DUP2}; -use crate::{fs::file_table::FileDescripter, log_syscall_entry, prelude::*}; +use crate::{ + fs::file_table::{FdFlags, FileDescripter}, + log_syscall_entry, + prelude::*, +}; pub fn sys_dup(old_fd: FileDescripter) -> Result { log_syscall_entry!(SYS_DUP); @@ -10,7 +14,8 @@ pub fn sys_dup(old_fd: FileDescripter) -> Result { let current = current!(); let mut file_table = current.file_table().lock(); let file = file_table.get_file(old_fd)?.clone(); - let new_fd = file_table.insert(file); + // The two file descriptors do not share the close-on-exec flag. + let new_fd = file_table.insert(file, FdFlags::empty()); Ok(SyscallReturn::Return(new_fd as _)) } @@ -22,7 +27,8 @@ pub fn sys_dup2(old_fd: FileDescripter, new_fd: FileDescripter) -> Result Result { log_syscall_entry!(SYS_EPOLL_CREATE1); debug!("flags = 0x{:x}", flags); - let close_on_exec = { + let fd_flags = { let flags = CreationFlags::from_bits(flags) .ok_or_else(|| Error::with_message(Errno::EINVAL, "invalid flags"))?; if flags == CreationFlags::empty() { - false + FdFlags::empty() } else if flags == CreationFlags::O_CLOEXEC { - true + FdFlags::CLOEXEC } else { // Only O_CLOEXEC is valid return_errno_with_message!(Errno::EINVAL, "invalid flags"); @@ -42,7 +42,7 @@ pub fn sys_epoll_create1(flags: u32) -> Result { let current = current!(); let epoll_file: Arc = EpollFile::new(); let mut file_table = current.file_table().lock(); - let fd = file_table.insert(epoll_file); + let fd = file_table.insert(epoll_file, fd_flags); Ok(SyscallReturn::Return(fd as _)) } diff --git a/kernel/aster-nix/src/syscall/execve.rs b/kernel/aster-nix/src/syscall/execve.rs index df5c0075a..2a32c1cc0 100644 --- a/kernel/aster-nix/src/syscall/execve.rs +++ b/kernel/aster-nix/src/syscall/execve.rs @@ -106,6 +106,12 @@ fn do_execve( let current = current!(); + // Ensure that the file descriptors with the close-on-exec flag are closed. + let closed_files = current.file_table().lock().close_files_on_exec(); + for file in closed_files { + file.clean_for_close()?; + } + debug!("load program to root vmar"); let (new_executable_path, elf_load_info) = { let fs_resolver = &*current.fs().read(); diff --git a/kernel/aster-nix/src/syscall/fcntl.rs b/kernel/aster-nix/src/syscall/fcntl.rs index ca81a9fda..664013fff 100644 --- a/kernel/aster-nix/src/syscall/fcntl.rs +++ b/kernel/aster-nix/src/syscall/fcntl.rs @@ -2,7 +2,10 @@ use super::{SyscallReturn, SYS_FCNTL}; use crate::{ - fs::{file_table::FileDescripter, utils::StatusFlags}, + fs::{ + file_table::{FdFlags, FileDescripter}, + utils::StatusFlags, + }, log_syscall_entry, prelude::*, }; @@ -12,18 +15,37 @@ pub fn sys_fcntl(fd: FileDescripter, cmd: i32, arg: u64) -> Result { - // FIXME: deal with the cloexec flag + FcntlCmd::F_DUPFD => { let current = current!(); let mut file_table = current.file_table().lock(); - let new_fd = file_table.dup(fd, arg as FileDescripter)?; + let new_fd = file_table.dup(fd, arg as FileDescripter, FdFlags::empty())?; Ok(SyscallReturn::Return(new_fd as _)) } + FcntlCmd::F_DUPFD_CLOEXEC => { + let current = current!(); + let mut file_table = current.file_table().lock(); + let new_fd = file_table.dup(fd, arg as FileDescripter, FdFlags::CLOEXEC)?; + Ok(SyscallReturn::Return(new_fd as _)) + } + FcntlCmd::F_GETFD => { + let current = current!(); + let file_table = current.file_table().lock(); + let entry = file_table.get_entry(fd)?; + let fd_flags = entry.flags(); + Ok(SyscallReturn::Return(fd_flags.bits() as _)) + } FcntlCmd::F_SETFD => { - if arg != 1 { - panic!("Unknown setfd argument"); - } - // TODO: Set cloexec + let flags = { + if arg > u8::MAX.into() { + return_errno_with_message!(Errno::EINVAL, "invalid fd flags"); + } + FdFlags::from_bits(arg as u8) + .ok_or(Error::with_message(Errno::EINVAL, "invalid flags"))? + }; + let current = current!(); + let file_table = current.file_table().lock(); + let entry = file_table.get_entry(fd)?; + entry.set_flags(flags); Ok(SyscallReturn::Return(0)) } FcntlCmd::F_GETFL => { @@ -60,7 +82,6 @@ pub fn sys_fcntl(fd: FileDescripter, cmd: i32, arg: u64) -> Result todo!(), } } diff --git a/kernel/aster-nix/src/syscall/open.rs b/kernel/aster-nix/src/syscall/open.rs index 928a41276..e5794a071 100644 --- a/kernel/aster-nix/src/syscall/open.rs +++ b/kernel/aster-nix/src/syscall/open.rs @@ -4,8 +4,9 @@ use super::{SyscallReturn, SYS_OPENAT}; use crate::{ fs::{ file_handle::FileLike, - file_table::FileDescripter, + file_table::{FdFlags, FileDescripter}, fs_resolver::{FsPath, AT_FDCWD}, + utils::CreationFlags, }, log_syscall_entry, prelude::*, @@ -35,7 +36,15 @@ pub fn sys_openat( Arc::new(inode_handle) }; let mut file_table = current.file_table().lock(); - let fd = file_table.insert(file_handle); + let fd = { + let fd_flags = + if CreationFlags::from_bits_truncate(flags).contains(CreationFlags::O_CLOEXEC) { + FdFlags::CLOEXEC + } else { + FdFlags::empty() + }; + file_table.insert(file_handle, fd_flags) + }; Ok(SyscallReturn::Return(fd as _)) } diff --git a/kernel/aster-nix/src/syscall/pipe.rs b/kernel/aster-nix/src/syscall/pipe.rs index bf21db5ac..035e8f962 100644 --- a/kernel/aster-nix/src/syscall/pipe.rs +++ b/kernel/aster-nix/src/syscall/pipe.rs @@ -3,9 +3,9 @@ use super::{SyscallReturn, SYS_PIPE2}; use crate::{ fs::{ - file_table::FileDescripter, + file_table::{FdFlags, FileDescripter}, pipe::{PipeReader, PipeWriter}, - utils::{Channel, StatusFlags}, + utils::{Channel, CreationFlags, StatusFlags}, }, log_syscall_entry, prelude::*, @@ -27,11 +27,16 @@ pub fn sys_pipe2(fds: Vaddr, flags: u32) -> Result { }; let pipe_reader = Arc::new(reader); let pipe_writer = Arc::new(writer); + let fd_flags = if CreationFlags::from_bits_truncate(flags).contains(CreationFlags::O_CLOEXEC) { + FdFlags::CLOEXEC + } else { + FdFlags::empty() + }; let current = current!(); let mut file_table = current.file_table().lock(); - pipe_fds.reader_fd = file_table.insert(pipe_reader); - pipe_fds.writer_fd = file_table.insert(pipe_writer); + pipe_fds.reader_fd = file_table.insert(pipe_reader, fd_flags); + pipe_fds.writer_fd = file_table.insert(pipe_writer, fd_flags); debug!("pipe_fds: {:?}", pipe_fds); write_val_to_user(fds, &pipe_fds)?; diff --git a/kernel/aster-nix/src/syscall/socket.rs b/kernel/aster-nix/src/syscall/socket.rs index 7673b8a52..34d6837a9 100644 --- a/kernel/aster-nix/src/syscall/socket.rs +++ b/kernel/aster-nix/src/syscall/socket.rs @@ -2,7 +2,7 @@ use super::{SyscallReturn, SYS_SOCKET}; use crate::{ - fs::file_handle::FileLike, + fs::{file_handle::FileLike, file_table::FdFlags}, log_syscall_entry, net::socket::{ ip::{DatagramSocket, StreamSocket}, @@ -42,7 +42,12 @@ pub fn sys_socket(domain: i32, type_: i32, protocol: i32) -> Result Resu let socket_fds = { let current = current!(); - let mut filetable = current.file_table().lock(); - let fd_a = filetable.insert(socket_a); - let fd_b = filetable.insert(socket_b); + let mut file_table = current.file_table().lock(); + let fd_flags = if sock_flags.contains(SockFlags::SOCK_CLOEXEC) { + FdFlags::CLOEXEC + } else { + FdFlags::empty() + }; + let fd_a = file_table.insert(socket_a, fd_flags); + let fd_b = file_table.insert(socket_b, fd_flags); SocketFds(fd_a, fd_b) };