-
Notifications
You must be signed in to change notification settings - Fork 70
Add support for getpgrp/getpgid/setpgid/TIOCGPGRP/TIOCSPGRP #640
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
57675c6
60514d0
bf807a4
7f62435
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -752,6 +752,7 @@ impl Task { | |
| pid: self.pid, | ||
| tid: child_tid, | ||
| ppid: self.ppid, | ||
| pgrp: self.pgrp.clone(), | ||
| credentials: self.credentials.clone(), | ||
| comm: self.comm.clone(), | ||
| fs: fs.into(), | ||
|
|
@@ -1148,10 +1149,47 @@ impl Task { | |
| self.pid | ||
| } | ||
|
|
||
| /// Handle syscall `getppid`. | ||
| pub(crate) fn sys_getppid(&self) -> i32 { | ||
| self.ppid | ||
| } | ||
|
|
||
| /// Handle syscall `getpgrp`. | ||
| pub(crate) fn sys_getpgrp(&self) -> i32 { | ||
| self.pgrp.load(core::sync::atomic::Ordering::Relaxed) | ||
| } | ||
|
|
||
| /// Handle syscall `getpgid`. | ||
| pub(crate) fn sys_getpgid(&self, pid: i32) -> Result<i32, Errno> { | ||
| if pid < 0 { | ||
| return Err(Errno::EINVAL); | ||
| } | ||
| if pid == 0 || pid == self.pid { | ||
| Ok(self.pgrp.load(core::sync::atomic::Ordering::Relaxed)) | ||
| } else { | ||
| Err(Errno::ESRCH) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add some comment here? Currently there is only process so we always return ESRCH. |
||
| } | ||
| } | ||
|
|
||
| /// Handle syscall `setpgid`. | ||
| #[allow(clippy::similar_names)] | ||
| pub(crate) fn sys_setpgid(&self, pid: i32, pgid: i32) -> Result<usize, Errno> { | ||
| if pid < 0 { | ||
| return Err(Errno::EINVAL); | ||
| } | ||
| let target_pid = if pid == 0 { self.pid } else { pid }; | ||
| let target_pgid = if pgid == 0 { target_pid } else { pgid }; | ||
| if target_pgid < 0 { | ||
| return Err(Errno::EINVAL); | ||
| } | ||
| if target_pid != self.pid { | ||
| return Err(Errno::ESRCH); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar |
||
| } | ||
| self.pgrp | ||
| .store(target_pgid, core::sync::atomic::Ordering::Relaxed); | ||
|
Comment on lines
+1188
to
+1189
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should add some check to ensure that |
||
| Ok(0) | ||
| } | ||
|
|
||
| /// Handle syscall `getuid`. | ||
| pub(crate) fn sys_getuid(&self) -> u32 { | ||
| self.credentials.uid | ||
|
|
@@ -1606,4 +1644,100 @@ mod tests { | |
| "prctl get_name returned unexpected comm for too long name" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_getpgrp() { | ||
| let task = crate::syscalls::tests::init_platform(None); | ||
|
|
||
| // getpgrp should return the pid (default behavior for new processes) | ||
| let pgrp = task.sys_getpgrp(); | ||
| let pid = task.sys_getpid(); | ||
| assert_eq!( | ||
| pgrp, pid, | ||
| "getpgrp should return pid for a new process that hasn't joined another process group" | ||
| ); | ||
|
|
||
| // After setpgid, getpgrp should reflect the new value | ||
| task.sys_setpgid(0, 42).expect("setpgid failed"); | ||
| assert_eq!(task.sys_getpgrp(), 42); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_getpgid_self() { | ||
| let task = crate::syscalls::tests::init_platform(None); | ||
| let pid = task.sys_getpid(); | ||
|
|
||
| // getpgid(0) should return own pgrp | ||
| assert_eq!(task.sys_getpgid(0).unwrap(), pid); | ||
| // getpgid(own_pid) should return own pgrp | ||
| assert_eq!(task.sys_getpgid(pid).unwrap(), pid); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_getpgid_unknown() { | ||
| use crate::Errno; | ||
| let task = crate::syscalls::tests::init_platform(None); | ||
|
|
||
| // getpgid for a negative pid should return EINVAL | ||
| assert_eq!(task.sys_getpgid(-1), Err(Errno::EINVAL)); | ||
|
|
||
| // getpgid for an unknown pid should return ESRCH | ||
| assert_eq!(task.sys_getpgid(99999), Err(Errno::ESRCH)); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_setpgid() { | ||
| let task = crate::syscalls::tests::init_platform(None); | ||
| let pid = task.sys_getpid(); | ||
|
|
||
| // setpgid(0, 0) is equivalent to setpgrp: sets pgrp to own pid | ||
| task.sys_setpgid(0, 0).expect("setpgid(0, 0) failed"); | ||
| assert_eq!(task.sys_getpgrp(), pid); | ||
| assert_eq!(task.sys_getpgid(0).unwrap(), pid); | ||
|
|
||
| // setpgid(0, 42) sets pgrp to 42 | ||
| task.sys_setpgid(0, 42).expect("setpgid(0, 42) failed"); | ||
| assert_eq!(task.sys_getpgrp(), 42); | ||
| assert_eq!(task.sys_getpgid(pid).unwrap(), 42); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_setpgid_invalid() { | ||
| use crate::Errno; | ||
| let task = crate::syscalls::tests::init_platform(None); | ||
|
|
||
| // Negative pid should return EINVAL | ||
| assert_eq!(task.sys_setpgid(-1, 1), Err(Errno::EINVAL)); | ||
|
|
||
| // Negative pgid should return EINVAL | ||
| assert_eq!(task.sys_setpgid(0, -1), Err(Errno::EINVAL)); | ||
|
|
||
| // Unknown pid should return ESRCH | ||
| assert_eq!(task.sys_setpgid(99999, 1), Err(Errno::ESRCH)); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_pgrp_inherited() { | ||
| let task = crate::syscalls::tests::init_platform(None); | ||
|
|
||
| // Set pgrp to a custom value | ||
| task.sys_setpgid(0, 42).expect("setpgid failed"); | ||
| assert_eq!(task.sys_getpgrp(), 42); | ||
|
|
||
| // Clone and verify child inherits the pgrp | ||
| let child = task.clone_for_test().expect("clone_for_test failed"); | ||
| assert_eq!(child.sys_getpgrp(), 42); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_pgrp_shared_between_threads() { | ||
| let task = crate::syscalls::tests::init_platform(None); | ||
| let child = task.clone_for_test().expect("clone_for_test failed"); | ||
|
|
||
| task.sys_setpgid(0, 42).expect("setpgid failed"); | ||
| assert_eq!(child.sys_getpgrp(), 42); | ||
|
|
||
| task.sys_setpgid(0, 7).expect("setpgid failed"); | ||
| assert_eq!(child.sys_getpgrp(), 7); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In LiteBox, there is no concept of forground and background process. Initializing it to 1 indicates that LiteBox always runs in foreground, but I can also run it in background (
cargo run -p litebox_runner_linux_userland ... &). In the latter case, LiteBox does not control the foreground process. Depending on how applications use this syscall, we may have different designs. For now, maybe we could just return some error code and print out some warnings.