From 50ba735e96715c8c40f62912df34b0921b589d4f Mon Sep 17 00:00:00 2001 From: Ruihan Li Date: Thu, 1 May 2025 23:51:37 +0800 Subject: [PATCH] Handle negative P(G)IDs via `cast_(un)signed` --- kernel/src/lib.rs | 1 + kernel/src/process/process_filter.rs | 60 +++++++++++++++------------- kernel/src/syscall/setpgid.rs | 4 ++ kernel/src/syscall/waitid.rs | 4 +- test/apps/process/group_session.c | 10 +++++ 5 files changed, 50 insertions(+), 29 deletions(-) diff --git a/kernel/src/lib.rs b/kernel/src/lib.rs index 35f9a8da..0608ff20 100644 --- a/kernel/src/lib.rs +++ b/kernel/src/lib.rs @@ -14,6 +14,7 @@ #![feature(fn_traits)] #![feature(format_args_nl)] #![feature(int_roundings)] +#![feature(integer_sign_cast)] #![feature(let_chains)] #![feature(linked_list_cursors)] #![feature(linked_list_remove)] diff --git a/kernel/src/process/process_filter.rs b/kernel/src/process/process_filter.rs index f33886cd..d8b436f9 100644 --- a/kernel/src/process/process_filter.rs +++ b/kernel/src/process/process_filter.rs @@ -1,7 +1,5 @@ // SPDX-License-Identifier: MPL-2.0 -#![expect(dead_code)] - use super::{Pgid, Pid}; use crate::prelude::*; @@ -13,44 +11,50 @@ pub enum ProcessFilter { } impl ProcessFilter { - // used for waitid - pub fn from_which_and_id(which: u64, id: u64) -> Result { - // Does not support PID_FD now(which = 3) - // https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/wait.h#L20 + // For `waitpid`. + pub fn from_which_and_id(which: u64, id: u32) -> Result { + // Reference: + // + const P_ALL: u64 = 0; + const P_PID: u64 = 1; + const P_PGID: u64 = 2; + const P_PIDFD: u64 = 3; + match which { - 0 => Ok(ProcessFilter::Any), - 1 => Ok(ProcessFilter::WithPid(id as Pid)), - 2 => Ok(ProcessFilter::WithPgid(id as Pgid)), - 3 => todo!(), - _ => return_errno_with_message!(Errno::EINVAL, "invalid which"), + P_ALL => Ok(ProcessFilter::Any), + P_PID => Ok(ProcessFilter::WithPid(id)), + P_PGID => Ok(ProcessFilter::WithPgid(id)), + P_PIDFD => { + warn!("the process filter `P_PIDFD` is not supported"); + return_errno_with_message!( + Errno::EINVAL, + "the process filter `P_PIDFD` is not supported" + ); + } + _ => return_errno_with_message!(Errno::EINVAL, "the process filter is invalid"), } } - // used for wait4 and kill + // For `wait4` and `kill`. pub fn from_id(wait_pid: i32) -> Self { - // https://man7.org/linux/man-pages/man2/waitpid.2.html - // https://man7.org/linux/man-pages/man2/kill.2.html + // Reference: + // + // if wait_pid < -1 { - // process group ID is equal to the absolute value of pid. - ProcessFilter::WithPgid((-wait_pid) as Pgid) + // "wait for any child process whose process group ID is equal to the absolute value of + // `pid`" + ProcessFilter::WithPgid((-wait_pid).cast_unsigned()) } else if wait_pid == -1 { - // wait for any child process + // "wait for any child process" ProcessFilter::Any } else if wait_pid == 0 { - // wait for any child process with same process group ID + // "wait for any child process whose process group ID is equal to that of the calling + // process at the time of the call to `waitpid()`" let pgid = current!().pgid(); ProcessFilter::WithPgid(pgid) } else { - // pid > 0. wait for the child whose process ID is equal to the value of pid. - ProcessFilter::WithPid(wait_pid as Pid) - } - } - - pub fn contains_pid(&self, pid: Pid) -> bool { - match self { - ProcessFilter::Any => true, - ProcessFilter::WithPid(filter_pid) => *filter_pid == pid, - ProcessFilter::WithPgid(_) => todo!(), + // "wait for the child whose process ID is equal to the value of `pid`" + ProcessFilter::WithPid(wait_pid.cast_unsigned()) } } } diff --git a/kernel/src/syscall/setpgid.rs b/kernel/src/syscall/setpgid.rs index 019af545..821c0ab0 100644 --- a/kernel/src/syscall/setpgid.rs +++ b/kernel/src/syscall/setpgid.rs @@ -12,6 +12,10 @@ pub fn sys_setpgid(pid: Pid, pgid: Pgid, ctx: &Context) -> Result // The documentation quoted below is from // . + if pid.cast_signed() < 0 || pgid.cast_signed() < 0 { + return_errno_with_message!(Errno::EINVAL, "negative PIDs or PGIDs are not valid"); + } + // "If `pid` is zero, then the process ID of the calling process is used." let pid = if pid == 0 { current.pid() } else { pid }; // "If `pgid` is zero, then the PGID of the process specified by `pid` is made the same as its diff --git a/kernel/src/syscall/waitid.rs b/kernel/src/syscall/waitid.rs index b13c1d20..ee7a594f 100644 --- a/kernel/src/syscall/waitid.rs +++ b/kernel/src/syscall/waitid.rs @@ -15,14 +15,16 @@ pub fn sys_waitid( ctx: &Context, ) -> Result { // FIXME: what does infoq and rusage use for? - let process_filter = ProcessFilter::from_which_and_id(which, upid)?; + let process_filter = ProcessFilter::from_which_and_id(which, upid as _)?; let wait_options = WaitOptions::from_bits(options as u32) .ok_or(Error::with_message(Errno::EINVAL, "invalid options"))?; + let waited_process = wait_child_exit(process_filter, wait_options, ctx).map_err(|err| match err.error() { Errno::EINTR => Error::new(Errno::ERESTARTSYS), _ => err, })?; + let pid = waited_process.map_or(0, |process| process.pid()); Ok(SyscallReturn::Return(pid as _)) } diff --git a/test/apps/process/group_session.c b/test/apps/process/group_session.c index ec09027f..9897b01b 100644 --- a/test/apps/process/group_session.c +++ b/test/apps/process/group_session.c @@ -32,6 +32,10 @@ END_SETUP() FN_TEST(setpgid_invalid) { + // Negative PIDs or PGIDs + TEST_ERRNO(setpgid(-1, current), EINVAL); + TEST_ERRNO(setpgid(current, -1), EINVAL); + // Non-present process groups TEST_ERRNO(setpgid(child1, child2), EPERM); TEST_ERRNO(setpgid(child2, child1), EPERM); @@ -176,6 +180,9 @@ END_TEST() FN_TEST(getpgid_invalid) { + // Negative PIDs + TEST_ERRNO(getpgid(-1), ESRCH); + // Non-present processes TEST_ERRNO(getpgid(0x3c3c3c3c), ESRCH); } @@ -183,6 +190,9 @@ END_TEST() FN_TEST(getsid_invalid) { + // Negative PIDs + TEST_ERRNO(getsid(-1), ESRCH); + // Non-present processes TEST_ERRNO(getsid(0x3c3c3c3c), ESRCH); }