diff --git a/kernel/src/process/clone.rs b/kernel/src/process/clone.rs index d38aacc1b..3facb9ddd 100644 --- a/kernel/src/process/clone.rs +++ b/kernel/src/process/clone.rs @@ -511,17 +511,23 @@ fn clone_sysvsem(clone_flags: CloneFlags) -> Result<()> { } fn set_parent_and_group(parent: &Process, child: &Arc) { - let process_group = parent.process_group().unwrap(); - + // Lock order: process table -> children -> group of process + // -> group inner -> session inner let mut process_table_mut = process_table::process_table_mut(); - let mut group_inner = process_group.inner.lock(); - let mut child_group_mut = child.process_group.lock(); - let mut children_mut = parent.children().lock(); + let mut children_mut = parent.children().lock(); + let process_group_mut = parent.process_group.lock(); + + let process_group = process_group_mut.upgrade().unwrap(); + let mut process_group_inner = process_group.lock(); + + // Put the child process in the parent's process group + process_group_inner.insert_process(child.clone()); + *child.process_group.lock() = Arc::downgrade(&process_group); + + // Put the child process in the parent's `children` field children_mut.insert(child.pid(), child.clone()); - group_inner.processes.insert(child.pid(), child.clone()); - *child_group_mut = Arc::downgrade(&process_group); - + // Put the child process in the global table process_table_mut.insert(child.pid(), child.clone()); } diff --git a/kernel/src/process/kill.rs b/kernel/src/process/kill.rs index 52fc4aff9..afa3c0f5e 100644 --- a/kernel/src/process/kill.rs +++ b/kernel/src/process/kill.rs @@ -54,8 +54,8 @@ pub fn kill_group(pgid: Pgid, signal: Option, ctx: &Context) -> Resu let process_group = process_table::get_process_group(&pgid) .ok_or_else(|| Error::with_message(Errno::ESRCH, "target group does not exist"))?; - let inner = process_group.inner.lock(); - for process in inner.processes.values() { + let inner = process_group.lock(); + for process in inner.iter() { kill_process(process, signal, ctx)?; } diff --git a/kernel/src/process/process/job_control.rs b/kernel/src/process/process/job_control.rs index 1395b5af1..ddefc056d 100644 --- a/kernel/src/process/process/job_control.rs +++ b/kernel/src/process/process/job_control.rs @@ -161,7 +161,8 @@ impl JobControl { return true; }; - foreground.contains_process(current!().pid()) + let foreground_inner = foreground.lock(); + foreground_inner.contains_process(¤t!().pid()) } } diff --git a/kernel/src/process/process/mod.rs b/kernel/src/process/process/mod.rs index 032738a7e..8bb3ad41c 100644 --- a/kernel/src/process/process/mod.rs +++ b/kernel/src/process/process/mod.rs @@ -243,37 +243,31 @@ impl Process { argv: Vec, envp: Vec, ) -> Result> { - let process_builder = { + let process = { let pid = allocate_posix_tid(); let parent = Weak::new(); - let credentials = Credentials::new_root(); let mut builder = ProcessBuilder::new(pid, executable_path, parent); builder.argv(argv).envp(envp).credentials(credentials); - builder + builder.build()? }; - let process = process_builder.build()?; - // Lock order: session table -> group table -> process table -> group of process - // -> group inner -> session inner let mut session_table_mut = process_table::session_table_mut(); let mut group_table_mut = process_table::group_table_mut(); let mut process_table_mut = process_table::process_table_mut(); - // Creates new group - let group = ProcessGroup::new(process.clone()); - *process.process_group.lock() = Arc::downgrade(&group); - group_table_mut.insert(group.pgid(), group.clone()); - - // Creates new session - let session = Session::new(group.clone()); - group.inner.lock().session = Arc::downgrade(&session); - session.inner.lock().leader = Some(process.clone()); - session_table_mut.insert(session.sid(), session); + // Create a new process group and a new session for the new process + process.set_new_session( + &mut process.process_group.lock(), + &mut session_table_mut, + &mut group_table_mut, + ); + // Insert the new process to the global table process_table_mut.insert(process.pid(), process.clone()); + Ok(process) } @@ -331,6 +325,7 @@ impl Process { } // *********** Parent and child *********** + pub fn parent(&self) -> &ParentProcess { &self.parent } @@ -343,254 +338,280 @@ impl Process { &self.children } - pub fn has_child(&self, pid: &Pid) -> bool { - self.children.lock().contains_key(pid) - } - pub fn children_wait_queue(&self) -> &WaitQueue { &self.children_wait_queue } - // *********** Process group & Session*********** + // *********** Process group & Session *********** - /// Returns the process group ID of the process. - pub fn pgid(&self) -> Pgid { - if let Some(process_group) = self.process_group.lock().upgrade() { - process_group.pgid() - } else { - 0 - } - } - - /// Returns the process group which the process belongs to. + /// Returns the process group to which the process belongs. pub fn process_group(&self) -> Option> { self.process_group.lock().upgrade() } - /// Returns whether `self` is the leader of process group. - fn is_group_leader(self: &Arc) -> bool { - let Some(process_group) = self.process_group() else { - return false; - }; - - let Some(leader) = process_group.leader() else { - return false; - }; - - Arc::ptr_eq(self, &leader) + /// Returns the process group ID of the process. + pub fn pgid(&self) -> Pgid { + self.process_group().map_or(0, |group| group.pgid()) } - /// Returns the session which the process belongs to. + /// Returns the session to which the process belongs. pub fn session(&self) -> Option> { - let process_group = self.process_group()?; - process_group.session() + self.process_group()?.session() } /// Returns whether the process is session leader. - pub fn is_session_leader(self: &Arc) -> bool { - let session = self.session().unwrap(); - - let Some(leading_process) = session.leader() else { - return false; - }; - - Arc::ptr_eq(self, &leading_process) + pub fn is_session_leader(&self) -> bool { + self.session() + .is_some_and(|session| session.sid() == self.pid) } /// Moves the process to the new session. /// - /// If the process is already session leader, this method does nothing. + /// This method will create a new process group in a new session, move the process to the new + /// session, and return the session ID (which is equal to the process ID and the process group + /// ID). /// - /// Otherwise, this method creates a new process group in a new session - /// and moves the process to the session, returning the new session. + /// # Errors /// - /// This method may return the following errors: - /// * `EPERM`, if the process is a process group leader, or some existing session - /// or process group has the same ID as the process. - pub fn to_new_session(self: &Arc) -> Result> { - if self.is_session_leader() { - return Ok(self.session().unwrap()); - } + /// This method will return `EPERM` if an existing process group has the same identifier as the + /// process ID. This means that the process is or was a process group leader and that the + /// process group is still alive. + pub fn to_new_session(self: &Arc) -> Result { + // Lock order: session table -> group table -> group of process + // -> group inner -> session inner + let mut session_table_mut = process_table::session_table_mut(); + let mut group_table_mut = process_table::group_table_mut(); - if self.is_group_leader() { + if session_table_mut.contains_key(&self.pid) { + // FIXME: According to the Linux implementation, this check should be removed, so we'll + // return `EPERM` due to hitting the following check. However, we need to work around a + // gVisor bug. The upstream gVisor has fixed the issue in: + // . + return Ok(self.pid); + } + if group_table_mut.contains_key(&self.pid) { return_errno_with_message!( Errno::EPERM, - "process group leader cannot be moved to new session." + "a process group leader cannot be moved to a new session" ); } - let session = self.session().unwrap(); + let mut process_group_mut = self.process_group.lock(); - // Lock order: session table -> group table -> group of process -> group inner -> session inner - let mut session_table_mut = process_table::session_table_mut(); - let mut group_table_mut = process_table::group_table_mut(); - let mut self_group_mut = self.process_group.lock(); + self.clear_old_group_and_session( + &mut process_group_mut, + &mut session_table_mut, + &mut group_table_mut, + ); - if session_table_mut.contains_key(&self.pid) { - return_errno_with_message!(Errno::EPERM, "cannot create new session"); - } - - if group_table_mut.contains_key(&self.pid) { - return_errno_with_message!(Errno::EPERM, "cannot create process group"); - } - - // Removes the process from old group - if let Some(old_group) = self_group_mut.upgrade() { - let mut group_inner = old_group.inner.lock(); - let mut session_inner = session.inner.lock(); - group_inner.remove_process(&self.pid); - *self_group_mut = Weak::new(); - - if group_inner.is_empty() { - group_table_mut.remove(&old_group.pgid()); - debug_assert!(session_inner.process_groups.contains_key(&old_group.pgid())); - session_inner.process_groups.remove(&old_group.pgid()); - - if session_inner.is_empty() { - session_table_mut.remove(&session.sid()); - } - } - } - - // Creates a new process group - let new_group = ProcessGroup::new(self.clone()); - *self_group_mut = Arc::downgrade(&new_group); - group_table_mut.insert(new_group.pgid(), new_group.clone()); - - // Creates a new session - let new_session = Session::new(new_group.clone()); - let mut new_group_inner = new_group.inner.lock(); - new_group_inner.session = Arc::downgrade(&new_session); - new_session.inner.lock().leader = Some(self.clone()); - session_table_mut.insert(new_session.sid(), new_session.clone()); - - // Removes the process from session. - let mut session_inner = session.inner.lock(); - session_inner.remove_process(self); - - Ok(new_session) + Ok(self.set_new_session( + &mut process_group_mut, + &mut session_table_mut, + &mut group_table_mut, + )) } - /// Moves the process to other process group. - /// - /// * If the group already exists, the process and the group should belong to the same session. - /// * If the group does not exist, this method creates a new group for the process and move the - /// process to the group. The group is added to the session of the process. - /// - /// This method may return `EPERM` in following cases: - /// * The process is session leader; - /// * The group already exists, but the group does not belong to the same session as the process; - /// * The group does not exist, but `pgid` is not equal to `pid` of the process. - pub fn to_other_group(self: &Arc, pgid: Pgid) -> Result<()> { - // if the process already belongs to the process group - if self.pgid() == pgid { - return Ok(()); - } + pub(super) fn clear_old_group_and_session( + &self, + process_group_mut: &mut MutexGuard>, + session_table_mut: &mut MutexGuard>>, + group_table_mut: &mut MutexGuard>>, + ) { + let process_group = process_group_mut.upgrade().unwrap(); + let mut process_group_inner = process_group.lock(); + let session = process_group.session().unwrap(); + let mut session_inner = session.lock(); - if self.is_session_leader() { - return_errno_with_message!(Errno::EPERM, "the process cannot be a session leader"); - } + // Remove the process from the process group. + process_group_inner.remove_process(&self.pid); + if process_group_inner.is_empty() { + group_table_mut.remove(&process_group.pgid()); - if let Some(process_group) = process_table::get_process_group(&pgid) { - let session = self.session().unwrap(); - if !session.contains_process_group(&process_group) { - return_errno_with_message!( - Errno::EPERM, - "the group and process does not belong to same session" - ); + // Remove the process group from the session. + session_inner.remove_process_group(&process_group.pgid()); + if session_inner.is_empty() { + session_table_mut.remove(&session.sid()); } - self.to_specified_group(&process_group)?; + } + + **process_group_mut = Weak::new(); + } + + fn set_new_session( + self: &Arc, + process_group_mut: &mut MutexGuard>, + session_table_mut: &mut MutexGuard>>, + group_table_mut: &mut MutexGuard>>, + ) -> Sid { + let (session, process_group) = Session::new_pair(self.clone()); + let sid = session.sid(); + + **process_group_mut = Arc::downgrade(&process_group); + + // Insert the new session and the new process group to the global table. + session_table_mut.insert(session.sid(), session); + group_table_mut.insert(process_group.pgid(), process_group); + + sid + } + + /// Moves the process itself or its child process to another process group. + /// + /// The process to be moved is specified with the process ID `pid`; `self` is used only for + /// permission checking purposes (see the Errors section below), which is typically + /// `current!()` when implementing system calls. + /// + /// If `pgid` is equal to the process ID, a new process group with the given PGID will be + /// created (if it does not already exist). Then, the process will be moved to the process + /// group with the given PGID, if the process group exists and belongs to the same session as + /// the given process. + /// + /// # Errors + /// + /// This method will return `ESRCH` in following cases: + /// * The process specified by `pid` does not exist; + /// * The process specified by `pid` is neither `self` or a child process of `self`. + /// + /// This method will return `EPERM` in following cases: + /// * The process is not in the same session as `self`; + /// * The process is a session leader, but the given PGID is not the process's PID/PGID; + /// * The process group already exists, but the group does not belong to the same session; + /// * The process group does not exist, but `pgid` is not equal to the process ID. + pub fn move_process_to_group(&self, pid: Pid, pgid: Pgid) -> Result<()> { + // Lock order: group table -> process table -> group of process + // -> group inner -> session inner + let group_table_mut = process_table::group_table_mut(); + let process_table_mut = process_table::process_table_mut(); + + let process = process_table_mut.get(pid).ok_or(Error::with_message( + Errno::ESRCH, + "the process to set the PGID does not exist", + ))?; + + let current_session = if self.pid == process.pid() { + // There is no need to check if the session is the same in this case. + None + } else if self.pid == process.parent().pid() { + // FIXME: If the child process has called `execve`, we should fail with `EACCESS`. + + // Immediately release the `self.process_group` lock to avoid deadlocks. Race + // conditions don't matter because this is used for comparison purposes only. + Some( + self.process_group + .lock() + .upgrade() + .unwrap() + .session() + .unwrap(), + ) } else { - if pgid != self.pid() { - return_errno_with_message!( - Errno::EPERM, - "the new process group should have the same ID as the process." - ); - } + return_errno_with_message!( + Errno::ESRCH, + "the process to set the PGID is neither the current process nor its child process" + ); + }; - self.to_new_group()?; + if let Some(new_process_group) = group_table_mut.get(&pgid).cloned() { + process.to_existing_group(current_session, group_table_mut, new_process_group) + } else if pgid == process.pid() { + process.to_new_group(current_session, group_table_mut) + } else { + return_errno_with_message!(Errno::EPERM, "the new process group does not exist"); } + } + + /// Moves the process to an existing group. + fn to_existing_group( + self: &Arc, + current_session: Option>, + mut group_table_mut: MutexGuard>>, + new_process_group: Arc, + ) -> Result<()> { + let mut process_group_mut = self.process_group.lock(); + let process_group = process_group_mut.upgrade().unwrap(); + + let session = process_group.session().unwrap(); + if session.sid() == self.pid { + return_errno_with_message!( + Errno::EPERM, + "a session leader cannot be moved to a new process group" + ); + } + if !Arc::ptr_eq(&session, &new_process_group.session().unwrap()) { + return_errno_with_message!( + Errno::EPERM, + "the new process group does not belong to the same session" + ); + } + if current_session.is_some_and(|current| !Arc::ptr_eq(¤t, &session)) { + return_errno_with_message!(Errno::EPERM, "the process belongs to a different session"); + } + + // Lock order: group with a smaller PGID -> group with a larger PGID + let (mut process_group_inner, mut new_group_inner) = + match process_group.pgid().cmp(&new_process_group.pgid()) { + core::cmp::Ordering::Less => { + let process_group_inner = process_group.lock(); + let new_group_inner = new_process_group.lock(); + (process_group_inner, new_group_inner) + } + core::cmp::Ordering::Greater => { + let new_group_inner = new_process_group.lock(); + let process_group_inner = process_group.lock(); + (process_group_inner, new_group_inner) + } + core::cmp::Ordering::Equal => return Ok(()), + }; + let mut session_inner = session.lock(); + + // Remove the process from the old process group + process_group_inner.remove_process(&self.pid); + if process_group_inner.is_empty() { + group_table_mut.remove(&process_group.pgid()); + session_inner.remove_process_group(&process_group.pgid()); + } + + // Insert the process to the new process group + new_group_inner.insert_process(self.clone()); + *process_group_mut = Arc::downgrade(&new_process_group); Ok(()) } /// Creates a new process group and moves the process to the group. - /// - /// The new group will be added to the same session as the process. - fn to_new_group(self: &Arc) -> Result<()> { - let session = self.session().unwrap(); - // Lock order: group table -> group of process -> group inner -> session inner - let mut group_table_mut = process_table::group_table_mut(); - let mut self_group_mut = self.process_group.lock(); + fn to_new_group( + self: &Arc, + current_session: Option>, + mut group_table_mut: MutexGuard>>, + ) -> Result<()> { + let mut process_group_mut = self.process_group.lock(); - // Removes the process from old group - if let Some(old_group) = self_group_mut.upgrade() { - let mut group_inner = old_group.inner.lock(); - let mut session_inner = session.inner.lock(); - group_inner.remove_process(&self.pid); - *self_group_mut = Weak::new(); + let process_group = process_group_mut.upgrade().unwrap(); + let session = process_group.session().unwrap(); - if group_inner.is_empty() { - group_table_mut.remove(&old_group.pgid()); - debug_assert!(session_inner.process_groups.contains_key(&old_group.pgid())); - // The old session won't be empty, since we will add a new group to the session. - session_inner.process_groups.remove(&old_group.pgid()); - } + if current_session.is_some_and(|current| !Arc::ptr_eq(¤t, &session)) { + return_errno_with_message!(Errno::EPERM, "the process belongs to a different session"); + } + if process_group.pgid() == self.pid { + // We'll hit this if the process is a session leader. There is no need to check below. + return Ok(()); } - // Creates a new process group. Adds the new group to group table and session. - let new_group = ProcessGroup::new(self.clone()); + let mut process_group_inner = process_group.lock(); + let mut session_inner = session.lock(); - let mut new_group_inner = new_group.inner.lock(); - let mut session_inner = session.inner.lock(); + // Remove the process from the old process group + process_group_inner.remove_process(&self.pid); + if process_group_inner.is_empty() { + group_table_mut.remove(&process_group.pgid()); + session_inner.remove_process_group(&process_group.pgid()); + } - *self_group_mut = Arc::downgrade(&new_group); - - group_table_mut.insert(new_group.pgid(), new_group.clone()); - - new_group_inner.session = Arc::downgrade(&session); - session_inner - .process_groups - .insert(new_group.pgid(), new_group.clone()); - - Ok(()) - } - - /// Moves the process to a specified group. - /// - /// The caller needs to ensure that the process and the group belongs to the same session. - fn to_specified_group(self: &Arc, group: &Arc) -> Result<()> { - // Lock order: group table -> group of process -> group inner (small pgid -> big pgid) - let mut group_table_mut = process_table::group_table_mut(); - let mut self_group_mut = self.process_group.lock(); - - // Removes the process from old group - let mut group_inner = if let Some(old_group) = self_group_mut.upgrade() { - // Lock order: group with smaller pgid first - let (mut old_group_inner, group_inner) = match old_group.pgid().cmp(&group.pgid()) { - core::cmp::Ordering::Equal => return Ok(()), - core::cmp::Ordering::Less => (old_group.inner.lock(), group.inner.lock()), - core::cmp::Ordering::Greater => { - let group_inner = group.inner.lock(); - let old_group_inner = old_group.inner.lock(); - (old_group_inner, group_inner) - } - }; - old_group_inner.remove_process(&self.pid); - *self_group_mut = Weak::new(); - - if old_group_inner.is_empty() { - group_table_mut.remove(&old_group.pgid()); - } - - group_inner - } else { - group.inner.lock() - }; - - // Adds the process to the specified group - group_inner.processes.insert(self.pid, self.clone()); - *self_group_mut = Arc::downgrade(group); + // Create a new process group and insert the process to it + let new_process_group = ProcessGroup::new(self.clone(), Arc::downgrade(&session)); + *process_group_mut = Arc::downgrade(&new_process_group); + group_table_mut.insert(new_process_group.pgid(), new_process_group.clone()); + session_inner.insert_process_group(new_process_group); Ok(()) } @@ -725,104 +746,3 @@ impl Process { } } } - -#[cfg(ktest)] -mod test { - - use ostd::prelude::*; - - use super::*; - - fn new_process(parent: Option>) -> Arc { - crate::util::random::init(); - crate::fs::rootfs::init_root_mount(); - let pid = allocate_posix_tid(); - let parent = if let Some(parent) = parent { - Arc::downgrade(&parent) - } else { - Weak::new() - }; - Process::new( - pid, - parent, - String::new(), - ProcessVm::alloc(), - ResourceLimits::default(), - Nice::default(), - Arc::new(Mutex::new(SigDispositions::default())), - ) - } - - fn new_process_in_session(parent: Option>) -> Arc { - // Lock order: session table -> group table -> group of process -> group inner - // -> session inner - let mut session_table_mut = process_table::session_table_mut(); - let mut group_table_mut = process_table::group_table_mut(); - - let process = new_process(parent); - // Creates new group - let group = ProcessGroup::new(process.clone()); - *process.process_group.lock() = Arc::downgrade(&group); - - // Creates new session - let sess = Session::new(group.clone()); - group.inner.lock().session = Arc::downgrade(&sess); - sess.inner.lock().leader = Some(process.clone()); - - group_table_mut.insert(group.pgid(), group); - session_table_mut.insert(sess.sid(), sess); - - process - } - - fn remove_session_and_group(process: Arc) { - // Lock order: session table -> group table - let mut session_table_mut = process_table::session_table_mut(); - let mut group_table_mut = process_table::group_table_mut(); - if let Some(sess) = process.session() { - session_table_mut.remove(&sess.sid()); - } - - if let Some(group) = process.process_group() { - group_table_mut.remove(&group.pgid()); - } - } - - #[ktest] - fn init_process() { - crate::time::clocks::init_for_ktest(); - let process = new_process(None); - assert!(process.process_group().is_none()); - assert!(process.session().is_none()); - } - - #[ktest] - fn init_process_in_session() { - crate::time::clocks::init_for_ktest(); - let process = new_process_in_session(None); - assert!(process.is_group_leader()); - assert!(process.is_session_leader()); - remove_session_and_group(process); - } - - #[ktest] - fn to_new_session() { - crate::time::clocks::init_for_ktest(); - let process = new_process_in_session(None); - let sess = process.session().unwrap(); - sess.inner.lock().leader = None; - - assert!(!process.is_session_leader()); - assert!(process - .to_new_session() - .is_err_and(|e| e.error() == Errno::EPERM)); - - let group = process.process_group().unwrap(); - group.inner.lock().leader = None; - assert!(!process.is_group_leader()); - - assert!(process - .to_new_session() - .is_err_and(|e| e.error() == Errno::EPERM)); - } -} diff --git a/kernel/src/process/process/process_group.rs b/kernel/src/process/process/process_group.rs index eb6662c00..9a89e1b1d 100644 --- a/kernel/src/process/process/process_group.rs +++ b/kernel/src/process/process/process_group.rs @@ -5,71 +5,53 @@ use alloc::collections::btree_map::Values; use super::{Pgid, Pid, Process, Session}; use crate::{prelude::*, process::signal::signals::Signal}; -/// `ProcessGroup` represents a set of processes. Each `ProcessGroup` has a unique -/// identifier `pgid`. +/// A process group. +/// +/// A process group represents a set of processes, +/// which has a unique identifier PGID (i.e., [`Pgid`]). pub struct ProcessGroup { pgid: Pgid, - pub(in crate::process) inner: Mutex, + session: Weak, + inner: Mutex, } -pub(in crate::process) struct Inner { - pub(in crate::process) processes: BTreeMap>, - pub(in crate::process) leader: Option>, - pub(in crate::process) session: Weak, -} - -impl Inner { - pub(in crate::process) fn remove_process(&mut self, pid: &Pid) { - let Some(process) = self.processes.remove(pid) else { - return; - }; - - if let Some(leader) = &self.leader - && Arc::ptr_eq(leader, &process) - { - self.leader = None; - } - } - - pub(in crate::process) fn is_empty(&self) -> bool { - self.processes.is_empty() - } +struct Inner { + processes: BTreeMap>, } impl ProcessGroup { - /// Creates a new process group with one process. The pgid is the same as the process - /// id. The process will become the leading process of the new process group. + /// Creates a new process group with one process. /// - /// The caller needs to ensure that the process does not belong to any group. - pub(in crate::process) fn new(process: Arc) -> Arc { + /// The PGID is the same as the process ID, which means that the process will become the leader + /// process of the new process group. + /// + /// The caller needs to ensure that the process does not belong to other process group. + pub(super) fn new(process: Arc, session: Weak) -> Arc { let pid = process.pid(); let inner = { let mut processes = BTreeMap::new(); - processes.insert(pid, process.clone()); - Inner { - processes, - leader: Some(process.clone()), - session: Weak::new(), - } + processes.insert(pid, process); + Inner { processes } }; Arc::new(ProcessGroup { pgid: pid, + session, inner: Mutex::new(inner), }) } - /// Returns whether self contains a process with `pid`. - pub(in crate::process) fn contains_process(&self, pid: Pid) -> bool { - self.inner.lock().processes.contains_key(&pid) - } - - /// Returns the process group identifier + /// Returns the process group identifier. pub fn pgid(&self) -> Pgid { self.pgid } + /// Returns the session to which the process group belongs. + pub fn session(&self) -> Option> { + self.session.upgrade() + } + /// Acquires a lock on the process group. pub fn lock(&self) -> ProcessGroupGuard { ProcessGroupGuard { @@ -77,29 +59,19 @@ impl ProcessGroup { } } - /// Broadcasts signal to all processes in the group. + /// Broadcasts the signal to all processes in the process group. /// - /// This method should only be used to broadcast fault signal and kernel signal. - /// - /// TODO: do more check to forbid user signal + /// This method should only be used to broadcast fault signals and kernel signals. + // + // TODO: Do some checks to forbid user signals. pub fn broadcast_signal(&self, signal: impl Signal + Clone + 'static) { for process in self.inner.lock().processes.values() { process.enqueue_signal(signal.clone()); } } - - /// Returns the leader process. - pub fn leader(&self) -> Option> { - self.inner.lock().leader.clone() - } - - /// Returns the session which the group belongs to - pub fn session(&self) -> Option> { - self.inner.lock().session.upgrade() - } } -/// A scoped lock for a process group. +/// A scoped lock guard for a process group. /// /// It provides some public methods to prevent the exposure of the inner type. #[clippy::has_significant_drop] @@ -109,12 +81,40 @@ pub struct ProcessGroupGuard<'a> { } impl ProcessGroupGuard<'_> { - /// Returns an iterator over the processes in the group. + /// Returns an iterator over the processes in the process group. pub fn iter(&self) -> ProcessGroupIter { ProcessGroupIter { inner: self.inner.processes.values(), } } + + /// Returns whether the process group contains the process. + pub(super) fn contains_process(&self, pid: &Pid) -> bool { + self.inner.processes.contains_key(pid) + } + + /// Inserts a process into the process group. + /// + /// The caller needs to ensure that the process didn't previously belong to the process group, + /// but now does. + pub(in crate::process) fn insert_process(&mut self, process: Arc) { + let old_process = self.inner.processes.insert(process.pid(), process); + debug_assert!(old_process.is_none()); + } + + /// Removes a process from the process group. + /// + /// The caller needs to ensure that the process previously belonged to the process group, but + /// now doesn't. + pub(in crate::process) fn remove_process(&mut self, pid: &Pid) { + let process = self.inner.processes.remove(pid); + debug_assert!(process.is_some()); + } + + /// Returns whether the process group is empty. + pub(in crate::process) fn is_empty(&self) -> bool { + self.inner.processes.is_empty() + } } /// An iterator over the processes of the process group. diff --git a/kernel/src/process/process/session.rs b/kernel/src/process/process/session.rs index 3e6121ca2..1e3c70c48 100644 --- a/kernel/src/process/process/session.rs +++ b/kernel/src/process/process/session.rs @@ -3,87 +3,72 @@ use super::{Pgid, Process, ProcessGroup, Sid, Terminal}; use crate::prelude::*; -/// A `Session` is a collection of related process groups. Each session has a -/// unique identifier `sid`. Process groups and sessions form a two-level -/// hierarchical relationship between processes. +/// A session. /// -/// **Leader**: A *session leader* is the process that creates a new session and whose process -/// ID becomes the session ID. +/// A session is a collection of related process groups, which has a unique identifier SID (i.e., +/// [`Sid`]). Process groups and sessions form a two-level hierarchical relationship between +/// processes. /// -/// **Controlling terminal**: The terminal can be used to manage all processes in the session. The +/// **Leader**: A *session leader* is the process that creates the session and whose process ID is +/// equal to the session ID. +/// +/// **Controlling terminal**: A terminal can be used to manage all processes in the session. The /// controlling terminal is established when the session leader first opens a terminal. pub struct Session { sid: Sid, - pub(in crate::process) inner: Mutex, + inner: Mutex, } -pub(in crate::process) struct Inner { - pub(in crate::process) process_groups: BTreeMap>, - pub(in crate::process) leader: Option>, - pub(in crate::process) terminal: Option>, -} - -impl Inner { - pub(in crate::process) fn is_empty(&self) -> bool { - self.process_groups.is_empty() - } - - pub(in crate::process) fn remove_process(&mut self, process: &Arc) { - if let Some(leader) = &self.leader - && Arc::ptr_eq(leader, process) - { - self.leader = None; - } - } - - pub(in crate::process) fn remove_process_group(&mut self, pgid: &Pgid) { - self.process_groups.remove(pgid); - } +struct Inner { + process_groups: BTreeMap>, + terminal: Option>, } impl Session { - /// Creates a new session for the process group. The process group becomes the member of - /// the new session. + /// Creates a new session and a new process group with one process. /// - /// The caller needs to ensure that the group does not belong to any session, and the caller - /// should set the leader process after creating the session. - pub(in crate::process) fn new(group: Arc) -> Arc { - let sid = group.pgid(); - let inner = { - let mut process_groups = BTreeMap::new(); - process_groups.insert(group.pgid(), group); + /// The SID and the PGID are the same as the process ID, which means that the process will + /// become the leader process of the new session and the new process group. + /// + /// The caller needs to ensure that the process does not belong to other process group or other + /// session. + pub(in crate::process) fn new_pair(process: Arc) -> (Arc, Arc) { + let mut process_group = None; - Inner { - process_groups, - leader: None, - terminal: None, + let session = Arc::new_cyclic(|weak_session| { + let group = ProcessGroup::new(process, weak_session.clone()); + process_group = Some(group.clone()); + + let pgid = group.pgid(); + + let inner = { + let mut process_groups = BTreeMap::new(); + process_groups.insert(pgid, group); + Inner { + process_groups, + terminal: None, + } + }; + + Self { + sid: pgid, + inner: Mutex::new(inner), } - }; - Arc::new(Self { - sid, - inner: Mutex::new(inner), - }) + }); + + (session, process_group.unwrap()) } - /// Returns the session id + /// Returns the session identifier. pub fn sid(&self) -> Sid { self.sid } - /// Returns the leader process. - pub fn leader(&self) -> Option> { - self.inner.lock().leader.clone() - } - - /// Returns whether `self` contains the `process_group` - pub(in crate::process) fn contains_process_group( - self: &Arc, - process_group: &Arc, - ) -> bool { - self.inner - .lock() - .process_groups - .contains_key(&process_group.pgid()) + /// Acquires a lock on the session. + pub fn lock(&self) -> SessionGuard { + SessionGuard { + inner: self.inner.lock(), + } } /// Sets terminal as the controlling terminal of the session. The `get_terminal` method @@ -134,3 +119,39 @@ impl Session { self.inner.lock().terminal.clone() } } + +/// A scoped lock guard for a session. +/// +/// It provides some public methods to prevent the exposure of the inner type. +#[clippy::has_significant_drop] +#[must_use] +pub struct SessionGuard<'a> { + inner: MutexGuard<'a, Inner>, +} + +impl SessionGuard<'_> { + /// Inserts a process group into the session. + /// + /// The caller needs to ensure that the process group didn't previously belong to the session, + /// but now does. + pub(in crate::process) fn insert_process_group(&mut self, process_group: Arc) { + let old_process_group = self + .inner + .process_groups + .insert(process_group.pgid(), process_group); + debug_assert!(old_process_group.is_none()); + } + + /// Removes a process group from the session. + /// + /// The caller needs to ensure that the process group previously belonged to the session, but + /// now doesn't. + pub(in crate::process) fn remove_process_group(&mut self, pgid: &Pgid) { + self.inner.process_groups.remove(pgid); + } + + /// Returns whether the session is empty. + pub(in crate::process) fn is_empty(&self) -> bool { + self.inner.process_groups.is_empty() + } +} diff --git a/kernel/src/process/wait.rs b/kernel/src/process/wait.rs index 6b492c496..1cd1df909 100644 --- a/kernel/src/process/wait.rs +++ b/kernel/src/process/wait.rs @@ -100,6 +100,7 @@ pub fn wait_child_exit( fn reap_zombie_child(process: &Process, pid: Pid) -> ExitCode { let child_process = process.children().lock().remove(&pid).unwrap(); assert!(child_process.status().is_zombie()); + for task in child_process.tasks().lock().as_slice() { thread_table::remove_thread(task.as_posix_thread().unwrap().tid()); } @@ -108,28 +109,19 @@ fn reap_zombie_child(process: &Process, pid: Pid) -> ExitCode { // -> group inner -> session inner let mut session_table_mut = process_table::session_table_mut(); let mut group_table_mut = process_table::group_table_mut(); + + // Remove the process from the global table let mut process_table_mut = process_table::process_table_mut(); + process_table_mut.remove(child_process.pid()); + // Remove the process group and the session from global table, if necessary let mut child_group_mut = child_process.process_group.lock(); - - let process_group = child_group_mut.upgrade().unwrap(); - let mut group_inner = process_group.inner.lock(); - let session = group_inner.session.upgrade().unwrap(); - let mut session_inner = session.inner.lock(); - - group_inner.remove_process(&child_process.pid()); - session_inner.remove_process(&child_process); + child_process.clear_old_group_and_session( + &mut child_group_mut, + &mut session_table_mut, + &mut group_table_mut, + ); *child_group_mut = Weak::new(); - if group_inner.is_empty() { - group_table_mut.remove(&process_group.pgid()); - session_inner.remove_process_group(&process_group.pgid()); - - if session_inner.is_empty() { - session_table_mut.remove(&session.sid()); - } - } - - process_table_mut.remove(child_process.pid()); child_process.status().exit_code() } diff --git a/kernel/src/syscall/setpgid.rs b/kernel/src/syscall/setpgid.rs index 991538a41..019af5454 100644 --- a/kernel/src/syscall/setpgid.rs +++ b/kernel/src/syscall/setpgid.rs @@ -3,35 +3,24 @@ use super::SyscallReturn; use crate::{ prelude::*, - process::{process_table, Pgid, Pid}, + process::{Pgid, Pid}, }; pub fn sys_setpgid(pid: Pid, pgid: Pgid, ctx: &Context) -> Result { let current = ctx.process; - // if pid is 0, pid should be the pid of current process + + // The documentation quoted below is from + // . + + // "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 0, pgid should be pid + // "If `pgid` is zero, then the PGID of the process specified by `pid` is made the same as its + // process ID." let pgid = if pgid == 0 { pid } else { pgid }; + debug!("pid = {}, pgid = {}", pid, pgid); - if pid != current.pid() && !current.has_child(&pid) { - return_errno_with_message!( - Errno::ESRCH, - "cannot set pgid for process other than current or children of current" - ); - } - // FIXME: If pid is child process of current and already calls execve, should return error. - // How can we determine a child process has called execve? - - // only can move process to an existing group or self - if pgid != pid && !process_table::contain_process_group(&pgid) { - return_errno_with_message!(Errno::EPERM, "process group must exist"); - } - - let process = process_table::get_process(pid) - .ok_or(Error::with_message(Errno::ESRCH, "process does not exist"))?; - - process.to_other_group(pgid)?; + current.move_process_to_group(pid, pgid)?; Ok(SyscallReturn::Return(0)) } diff --git a/kernel/src/syscall/setsid.rs b/kernel/src/syscall/setsid.rs index c9a6e13a0..3cae252b6 100644 --- a/kernel/src/syscall/setsid.rs +++ b/kernel/src/syscall/setsid.rs @@ -4,8 +4,7 @@ use super::SyscallReturn; use crate::prelude::*; pub fn sys_setsid(_ctx: &Context) -> Result { - let current = current!(); - let session = current.to_new_session()?; + let sid = current!().to_new_session()?; - Ok(SyscallReturn::Return(session.sid() as _)) + Ok(SyscallReturn::Return(sid as _)) } diff --git a/test/apps/Makefile b/test/apps/Makefile index e4e60f1e2..0894e9bca 100644 --- a/test/apps/Makefile +++ b/test/apps/Makefile @@ -34,6 +34,7 @@ TEST_APPS := \ network \ pipe \ prctl \ + process \ pthread \ pty \ sched \ diff --git a/test/apps/process/Makefile b/test/apps/process/Makefile new file mode 100644 index 000000000..c603a781a --- /dev/null +++ b/test/apps/process/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: MPL-2.0 + +include ../test_common.mk + +EXTRA_C_FLAGS := diff --git a/test/apps/process/group_session.c b/test/apps/process/group_session.c new file mode 100644 index 000000000..875ea3325 --- /dev/null +++ b/test/apps/process/group_session.c @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: MPL-2.0 + +#include "../network/test.h" + +#include +#include + +static pid_t current; +static pid_t child1, child2; + +FN_SETUP(setpgrp) +{ + CHECK(setpgid(0, 0)); +} +END_SETUP() + +FN_SETUP(spawn_child) +{ + current = CHECK(getpid()); + + if ((child1 = CHECK(fork())) == 0) { + sleep(60); + exit(EXIT_FAILURE); + } + + if ((child2 = CHECK(fork())) == 0) { + sleep(60); + exit(EXIT_FAILURE); + } +} +END_SETUP() + +FN_TEST(setpgid_invalid) +{ + // Non-present process groups + TEST_ERRNO(setpgid(child1, child2), EPERM); + TEST_ERRNO(setpgid(child2, child1), EPERM); + TEST_ERRNO(setpgid(child1, 0x3c3c3c3c), EPERM); + TEST_ERRNO(setpgid(child2, 0x3c3c3c3c), EPERM); + + // Non-present processes + TEST_ERRNO(setpgid(0x3c3c3c3c, current), ESRCH); + TEST_ERRNO(setpgid(0x3c3c3c3c, current), ESRCH); + + // Non-current and non-child processes + TEST_ERRNO(setpgid(getppid(), 0), ESRCH); + TEST_ERRNO(setpgid(getppid(), 0x3c3c3c3c), ESRCH); +} +END_TEST() + +FN_TEST(setpgid) +{ + // PGID members + // | | + // v v + // Process groups: [current] = { current, child1, child2 } + + TEST_SUCC(setpgid(0, 0)); + TEST_SUCC(setpgid(0, current)); + TEST_SUCC(setpgid(current, 0)); + TEST_SUCC(setpgid(current, getpid())); + + TEST_ERRNO(setpgid(child1, child2), EPERM); + TEST_ERRNO(setpgid(child2, child1), EPERM); + + // Process groups: [current] = { current, child2 }, [child1] = { child1 } + TEST_SUCC(setpgid(child1, 0)); + + // Process groups: [current] = { current }, [child1] = { child1, child2 } + TEST_SUCC(setpgid(child2, child1)); + + // Process groups: [current] = { current, child1 }, [child1] = { child2 } + TEST_SUCC(setpgid(child1, current)); + + // Process groups: [current] = { current }, [child1] = { child1, child2 } + TEST_SUCC(setpgid(child1, child1)); +} +END_TEST() + +FN_TEST(setsid_group_leader) +{ + // Process groups: [current] = { current }, [child1] = { child1, child2 } + + TEST_ERRNO(setsid(), EPERM); + + // Process groups: [current] = { child1 }, [child1] = { current, child2 } + TEST_SUCC(setpgid(child1, current)); + TEST_SUCC(setpgid(current, child1)); + + TEST_ERRNO(setsid(), EPERM); +} +END_TEST() + +FN_TEST(setsid) +{ + // Process groups: [child1] = { current, child1, child2 } + TEST_SUCC(setpgid(child1, child1)); + + // Process groups (old session): [child1] = { child1, child2 } + // Process groups (new session): [current] = { current } + TEST_SUCC(setsid()); +} +END_TEST() + +// From now on, the current process and the child processes are in two sessions! + +FN_TEST(setsid_session_leader) +{ + // FIXME: We fail this test to work around a gVisor bug. + // See comments in `Process::to_new_session` for details. + // + // TEST_ERRNO(setsid(), EPERM); +} +END_TEST() + +FN_TEST(setpgid_two_sessions) +{ + // Setting process groups in another session should never succeed + + TEST_ERRNO(setpgid(child1, child1), EPERM); + TEST_ERRNO(setpgid(child2, child2), EPERM); + + TEST_ERRNO(setpgid(child1, current), EPERM); + TEST_ERRNO(setpgid(child2, current), EPERM); + + TEST_ERRNO(setpgid(child2, child1), EPERM); +} +END_TEST() + +FN_SETUP(kill_child) +{ + CHECK(kill(child1, SIGKILL)); + CHECK_WITH(wait(NULL), _ret == child1); + + CHECK(kill(child2, SIGKILL)); + CHECK_WITH(wait(NULL), _ret == child2); +} +END_SETUP() diff --git a/test/apps/scripts/process.sh b/test/apps/scripts/process.sh index 0de8608b2..131be32f4 100755 --- a/test/apps/scripts/process.sh +++ b/test/apps/scripts/process.sh @@ -29,6 +29,7 @@ itimer/timer_create mmap/mmap_and_fork mmap/mmap_shared_filebacked mmap/mmap_readahead +process/group_session pthread/pthread_test pty/open_pty sched/sched_attr