diff --git a/kernel/src/syscall/sched_getattr.rs b/kernel/src/syscall/sched_getattr.rs index 55f79b665..3075e8d9b 100644 --- a/kernel/src/syscall/sched_getattr.rs +++ b/kernel/src/syscall/sched_getattr.rs @@ -1,5 +1,7 @@ // SPDX-License-Identifier: MPL-2.0 +use ostd::const_assert; + use super::{ sched_get_priority_max::{rt_to_static, static_to_rt, SCHED_PRIORITY_RANGE}, SyscallReturn, @@ -14,11 +16,11 @@ use crate::{ pub(super) const SCHED_NORMAL: u32 = 0; pub(super) const SCHED_FIFO: u32 = 1; pub(super) const SCHED_RR: u32 = 2; -// pub(super) const SCHED_BATCH: u32 = 3; // not supported (never). -// SCHED_ISO: reserved but not implemented yet on Linux. +// pub(super) const SCHED_BATCH: u32 = 3; // Not supported. +// SCHED_ISO: Reserved but not implemented yet on Linux. pub(super) const SCHED_IDLE: u32 = 5; -// pub(super) const SCHED_DEADLINE: u32 = 6; // not supported yet. -// pub(super) const SCHED_EXT: u32 = 7; // not supported (never). +// pub(super) const SCHED_DEADLINE: u32 = 6; // Not supported. +// pub(super) const SCHED_EXT: u32 = 7; // Not supported. #[derive(Default, Debug, Pod, Clone, Copy)] #[repr(C)] @@ -47,6 +49,14 @@ pub(super) struct LinuxSchedAttr { pub(super) sched_util_max: u32, } +// Reference: +const SCHED_ATTR_SIZE_VER0: u32 = 48; +// Reference: +#[cfg_attr(target_arch = "x86_64", expect(dead_code))] +const SCHED_ATTR_SIZE_VER1: u32 = 56; + +const_assert!(size_of::() == SCHED_ATTR_SIZE_VER1 as usize); + impl TryFrom for LinuxSchedAttr { type Error = Error; @@ -81,12 +91,10 @@ impl TryFrom for LinuxSchedAttr { ..Default::default() }, - SchedPolicy::Idle => { - return Err(Error::with_message( - Errno::EACCES, - "attr for idle tasks are not accessible", - )) - } + SchedPolicy::Idle => return_errno_with_message!( + Errno::EACCES, + "scheduling attributes for idle tasks are not accessible" + ), }) } } @@ -108,16 +116,14 @@ impl TryFrom for SchedPolicy { }, _ if value.sched_priority != 0 => { - return Err(Error::with_message( - Errno::EINVAL, - "Invalid scheduling priority", - )) + return_errno_with_message!(Errno::EINVAL, "invalid scheduling priority") } SCHED_NORMAL => SchedPolicy::Fair(Nice::new( - i8::try_from(value.sched_nice)? - .try_into() - .map_err(|msg| Error::with_message(Errno::EINVAL, msg))?, + i8::try_from(value.sched_nice) + .ok() + .and_then(|n| n.try_into().ok()) + .ok_or_else(|| Error::with_message(Errno::EINVAL, "invalid nice number"))?, )), // The SCHED_IDLE policy is mapped to the highest nice value of @@ -125,12 +131,7 @@ impl TryFrom for SchedPolicy { // latter policy are invisible to the user API. SCHED_IDLE => SchedPolicy::Fair(Nice::MAX), - _ => { - return Err(Error::with_message( - Errno::EINVAL, - "Invalid scheduling policy", - )) - } + _ => return_errno_with_message!(Errno::EINVAL, "invalid scheduling policy"), }) } } @@ -139,28 +140,47 @@ pub(super) fn read_linux_sched_attr_from_user( addr: Vaddr, ctx: &Context, ) -> Result { - let type_size = size_of::(); + // The code below is written according to the Linux implementation. + // Reference: - let space = ctx.user_space(); + let user_space = ctx.user_space(); - let mut attr = LinuxSchedAttr::default(); + let raw_size = user_space.read_val::(addr)?; + let size = if raw_size == 0 { + SCHED_ATTR_SIZE_VER0 + } else { + raw_size + }; + if size < SCHED_ATTR_SIZE_VER0 || size > PAGE_SIZE as u32 { + let _ = user_space.write_val(addr, &(size_of::() as u32)); + return_errno_with_message!(Errno::E2BIG, "invalid scheduling attribute size"); + } - space.read_bytes( - addr, - &mut VmWriter::from(&mut attr.as_bytes_mut()[..size_of::()]), - )?; + let mut attr = LinuxSchedAttr { + size, + ..Default::default() + }; - let size = type_size.min(attr.size as usize); - space.read_bytes(addr, &mut VmWriter::from(&mut attr.as_bytes_mut()[..size]))?; + let mut reader = user_space.reader(addr, size as usize)?; + reader.skip(size_of::()); + reader.read_fallible(&mut VmWriter::from( + &mut attr.as_bytes_mut()[size_of::()..], + ))?; - if let Some(additional_size) = attr.size.checked_sub(type_size as u32) { - let mut buf = vec![0; additional_size as usize]; - space.read_bytes(addr + type_size, &mut VmWriter::from(&mut *buf))?; - - if buf.iter().any(|&b| b != 0) { - return Err(Error::with_message(Errno::E2BIG, "too big sched_attr")); + while reader.remain() > size_of::() { + if reader.read_val::()? != 0 { + let _ = user_space.write_val(addr, &(size_of::() as u32)); + return_errno_with_message!(Errno::E2BIG, "incompatible scheduling attributes"); } } + while reader.has_remain() { + if reader.read_val::()? != 0 { + let _ = user_space.write_val(addr, &(size_of::() as u32)); + return_errno_with_message!(Errno::E2BIG, "incompatible scheduling attributes"); + } + } + + // TODO: Check whether `sched_flags` is valid. Ok(attr) } @@ -171,20 +191,22 @@ pub(super) fn write_linux_sched_attr_to_user( user_size: u32, ctx: &Context, ) -> Result<()> { - let space = ctx.user_space(); + if user_size < SCHED_ATTR_SIZE_VER0 || user_size > PAGE_SIZE as u32 { + return_errno_with_message!(Errno::EINVAL, "invalid scheduling attribute size"); + } attr.size = (size_of::() as u32).min(user_size); - let range = SCHED_PRIORITY_RANGE - .get(attr.sched_policy as usize) - .ok_or_else(|| Error::with_message(Errno::EINVAL, "invalid scheduling policy"))?; + let range = &SCHED_PRIORITY_RANGE[attr.sched_policy as usize]; attr.sched_util_min = *range.start(); attr.sched_util_max = *range.end(); - space.write_bytes( + let user_space = ctx.user_space(); + user_space.write_bytes( addr, &mut VmReader::from(&attr.as_bytes()[..attr.size as usize]), )?; + Ok(()) } @@ -193,13 +215,18 @@ pub(super) fn access_sched_attr_with( ctx: &Context, f: impl FnOnce(&SchedAttr) -> Result, ) -> Result { - match tid { - 0 => f(ctx.thread.sched_attr()), - _ if tid > (i32::MAX as u32) => Err(Error::with_message(Errno::EINVAL, "invalid tid")), - _ => f(thread_table::get_thread(tid) - .ok_or_else(|| Error::with_message(Errno::ESRCH, "thread does not exist"))? - .sched_attr()), + if tid.cast_signed() < 0 { + return_errno_with_message!(Errno::EINVAL, "all negative TIDs are not valid"); } + + if tid == 0 { + return f(ctx.thread.sched_attr()); + } + + let Some(thread) = thread_table::get_thread(tid) else { + return_errno_with_message!(Errno::ESRCH, "the target thread does not exist"); + }; + f(thread.sched_attr()) } pub fn sys_sched_getattr( @@ -209,15 +236,19 @@ pub fn sys_sched_getattr( flags: u32, ctx: &Context, ) -> Result { + if addr == 0 { + return_errno_with_message!(Errno::EINVAL, "invalid user space address"); + } if flags != 0 { - // TODO: support flags soch as `RESET_ON_FORK`. - return Err(Error::with_message(Errno::EINVAL, "unsupported flags")); + // Linux also has no support for any flags yet. + return_errno_with_message!(Errno::EINVAL, "invalid flags"); } let policy = access_sched_attr_with(tid, ctx, |attr| Ok(attr.policy()))?; - let attr: LinuxSchedAttr = policy.try_into()?; - write_linux_sched_attr_to_user(attr, addr, user_size, ctx) - .map_err(|_| Error::new(Errno::EINVAL))?; + let attr: LinuxSchedAttr = policy + .try_into() + .expect("all user-visible scheduling attributes should be valid"); + write_linux_sched_attr_to_user(attr, addr, user_size, ctx)?; Ok(SyscallReturn::Return(0)) } diff --git a/kernel/src/syscall/sched_getparam.rs b/kernel/src/syscall/sched_getparam.rs index 07d5f8947..04281c6cd 100644 --- a/kernel/src/syscall/sched_getparam.rs +++ b/kernel/src/syscall/sched_getparam.rs @@ -4,16 +4,17 @@ use super::{sched_getattr::access_sched_attr_with, SyscallReturn}; use crate::{prelude::*, sched::SchedPolicy, thread::Tid}; pub fn sys_sched_getparam(tid: Tid, addr: Vaddr, ctx: &Context) -> Result { + if addr == 0 { + return_errno_with_message!(Errno::EINVAL, "invalid user space address"); + } + let policy = access_sched_attr_with(tid, ctx, |attr| Ok(attr.policy()))?; let rt_prio = match policy { SchedPolicy::RealTime { rt_prio, .. } => rt_prio.get().into(), _ => 0, }; - let space = ctx.user_space(); - space - .write_val(addr, &rt_prio) - .map_err(|_| Error::new(Errno::EINVAL))?; + ctx.user_space().write_val(addr, &rt_prio)?; Ok(SyscallReturn::Return(0)) } diff --git a/kernel/src/syscall/sched_setattr.rs b/kernel/src/syscall/sched_setattr.rs index bf5140a9f..8ecb78f8f 100644 --- a/kernel/src/syscall/sched_setattr.rs +++ b/kernel/src/syscall/sched_setattr.rs @@ -12,12 +12,15 @@ pub fn sys_sched_setattr( flags: u32, ctx: &Context, ) -> Result { + if addr == 0 { + return_errno_with_message!(Errno::EINVAL, "invalid user space address"); + } if flags != 0 { - // TODO: support flags soch as `RESET_ON_FORK`. - return Err(Error::with_message(Errno::EINVAL, "unsupported flags")); + // Linux also has no support for any flags yet. + return_errno_with_message!(Errno::EINVAL, "invalid flags"); } - let attr = read_linux_sched_attr_from_user(addr, ctx).map_err(|_| Error::new(Errno::EINVAL))?; + let attr = read_linux_sched_attr_from_user(addr, ctx)?; let policy = SchedPolicy::try_from(attr)?; access_sched_attr_with(tid, ctx, |attr| { attr.set_policy(policy); diff --git a/kernel/src/syscall/sched_setparam.rs b/kernel/src/syscall/sched_setparam.rs index 0b9c4413c..dcaa805a9 100644 --- a/kernel/src/syscall/sched_setparam.rs +++ b/kernel/src/syscall/sched_setparam.rs @@ -4,19 +4,25 @@ use super::{sched_getattr::access_sched_attr_with, SyscallReturn}; use crate::{prelude::*, sched::SchedPolicy, thread::Tid}; pub fn sys_sched_setparam(tid: Tid, addr: Vaddr, ctx: &Context) -> Result { - let space = ctx.user_space(); - let prio: i32 = space - .read_val(addr) - .map_err(|_| Error::new(Errno::EINVAL))?; + if addr == 0 { + return_errno_with_message!(Errno::EINVAL, "invalid user space address"); + } + + let prio: i32 = ctx.user_space().read_val(addr)?; let update = |policy: &mut SchedPolicy| { match policy { SchedPolicy::RealTime { rt_prio, .. } => { - *rt_prio = u8::try_from(prio)? - .try_into() - .map_err(|msg| Error::with_message(Errno::EINVAL, msg))?; + *rt_prio = u8::try_from(prio) + .ok() + .and_then(|p| p.try_into().ok()) + .ok_or_else(|| { + Error::with_message(Errno::EINVAL, "invalid scheduling priority") + })?; + } + _ if prio != 0 => { + return_errno_with_message!(Errno::EINVAL, "invalid scheduling priority") } - _ if prio != 0 => return Err(Error::with_message(Errno::EINVAL, "invalid priority")), _ => {} } Ok(()) diff --git a/kernel/src/syscall/sched_setscheduler.rs b/kernel/src/syscall/sched_setscheduler.rs index 86bf7f82e..86ea2f343 100644 --- a/kernel/src/syscall/sched_setscheduler.rs +++ b/kernel/src/syscall/sched_setscheduler.rs @@ -12,10 +12,11 @@ pub fn sys_sched_setscheduler( addr: Vaddr, ctx: &Context, ) -> Result { - let space = ctx.user_space(); - let prio = space - .read_val(addr) - .map_err(|_| Error::new(Errno::EINVAL))?; + if addr == 0 { + return_errno_with_message!(Errno::EINVAL, "invalid user space address"); + } + + let prio = ctx.user_space().read_val(addr)?; let attr = LinuxSchedAttr { sched_policy: policy as u32, diff --git a/test/src/apps/sched/sched_attr_getset.c b/test/src/apps/sched/sched_attr_getset.c new file mode 100644 index 000000000..908756366 --- /dev/null +++ b/test/src/apps/sched/sched_attr_getset.c @@ -0,0 +1,70 @@ +// SPDX-License-Identifier: MPL-2.0 + +#include +#include +#include + +#include "../test.h" + +static int sched_setattr(pid_t pid, struct sched_attr *attr, unsigned int flags) +{ + return syscall(SYS_sched_setattr, pid, attr, flags); +} + +static int sched_getattr(pid_t pid, struct sched_attr *attr, unsigned int size, + unsigned int flags) +{ + return syscall(SYS_sched_getattr, pid, attr, size, flags); +} + +FN_TEST(sched_attr) +{ +#define PAGE_SIZE 4096 +#define TAIL_LEN 13 + union { + struct sched_attr sched; + char buf[sizeof(struct sched_attr) + TAIL_LEN]; + } attr; + + memset(attr.buf, 0, sizeof(attr.buf)); + + // Test `sched_getattr` with invalid sizes. + TEST_ERRNO(sched_getattr(0, &attr.sched, SCHED_ATTR_SIZE_VER0 - 1, 0), + EINVAL); + TEST_ERRNO(sched_getattr(0, &attr.sched, PAGE_SIZE + 1, 0), EINVAL); + + // Test `sched_getattr` with valid sizes. + TEST_ERRNO(sched_getattr(0, &attr.sched, SCHED_ATTR_SIZE_VER0, 0), + attr.sched.size == SCHED_ATTR_SIZE_VER1); + TEST_RES(sched_getattr(0, &attr.sched, sizeof(attr.sched), 0), + attr.sched.size == SCHED_ATTR_SIZE_VER1); + + // Test `sched_setattr` with invalid sizes. + attr.sched.size = SCHED_ATTR_SIZE_VER0 - 1; + TEST(sched_setattr(0, &attr.sched, 0), E2BIG, + attr.sched.size == SCHED_ATTR_SIZE_VER1); + attr.sched.size = PAGE_SIZE + 1; + TEST(sched_setattr(0, &attr.sched, 0), E2BIG, + attr.sched.size == SCHED_ATTR_SIZE_VER1); + + // Test `sched_setattr` with valid sizes. + attr.sched.size = SCHED_ATTR_SIZE_VER0; + TEST_SUCC(sched_setattr(0, &attr.sched, 0)); + attr.sched.size = SCHED_ATTR_SIZE_VER1; + TEST_SUCC(sched_setattr(0, &attr.sched, 0)); + attr.sched.size = sizeof(attr.buf); + TEST_SUCC(sched_setattr(0, &attr.sched, 0)); + + // Test `sched_setattr` with valid sizes, but garbage trailing data. + for (int i = 0; i < TAIL_LEN; ++i) { + attr.buf[sizeof(struct sched_attr) + i] = i + 1; + + attr.sched.size = sizeof(attr.buf); + // The size is updated even if `sched_setattr` fails with `E2BIG`. + TEST(sched_setattr(0, &attr.sched, 0), E2BIG, + attr.sched.size == SCHED_ATTR_SIZE_VER1); + + attr.buf[sizeof(struct sched_attr) + i] = 0; + } +} +END_TEST() diff --git a/test/src/apps/sched/sched_attr.c b/test/src/apps/sched/sched_param_getset.c similarity index 89% rename from test/src/apps/sched/sched_attr.c rename to test/src/apps/sched/sched_param_getset.c index 8960193f6..86dee3cc2 100644 --- a/test/src/apps/sched/sched_attr.c +++ b/test/src/apps/sched/sched_param_getset.c @@ -2,36 +2,32 @@ #define _GNU_SOURCE -#include "../test.h" -#include -#include -#include #include +#include -FN_SETUP() -{ -} -END_SETUP() +#include "../test.h" FN_TEST(sched_param) { + struct sched_param param; + TEST_ERRNO(sched_getscheduler(-100), EINVAL); TEST_ERRNO(sched_getscheduler(1234567890), ESRCH); TEST_RES(sched_getscheduler(0), _ret == SCHED_OTHER); - struct sched_param param; - TEST_ERRNO(sched_getparam(0, NULL), EINVAL); + TEST_ERRNO(sched_getparam(0, (void *)1), EFAULT); TEST_RES(sched_getparam(0, ¶m), _ret == 0 && param.sched_priority == 0); param.sched_priority = 50; TEST_ERRNO(sched_setscheduler(0, SCHED_FIFO, NULL), EINVAL); + TEST_ERRNO(sched_setscheduler(0, SCHED_FIFO, (void *)1), EFAULT); TEST_ERRNO(sched_setscheduler(0, 1234567890, ¶m), EINVAL); TEST_ERRNO(sched_setscheduler(-100, SCHED_FIFO, ¶m), EINVAL); TEST_ERRNO(sched_setscheduler(1234567890, SCHED_FIFO, ¶m), ESRCH); TEST_RES(sched_setscheduler(0, SCHED_FIFO, ¶m), _ret == 0); - sleep(1); + usleep(200 * 1000); TEST_RES(sched_getscheduler(0), _ret == SCHED_FIFO); TEST_RES(sched_getparam(0, ¶m), @@ -39,12 +35,13 @@ FN_TEST(sched_param) param.sched_priority = 1234567890; TEST_ERRNO(sched_setparam(0, NULL), EINVAL); + TEST_ERRNO(sched_setparam(0, (void *)1), EFAULT); TEST_ERRNO(sched_setparam(-100, ¶m), EINVAL); TEST_ERRNO(sched_setparam(1234567890, ¶m), ESRCH); TEST_ERRNO(sched_setparam(0, ¶m), EINVAL); param.sched_priority = 51; TEST_RES(sched_setparam(0, ¶m), _ret == 0); - sleep(1); + usleep(200 * 1000); TEST_RES(sched_getparam(0, ¶m), _ret == 0 && param.sched_priority == 51); @@ -72,4 +69,4 @@ FN_TEST(sched_prio_limit) TEST_RES(sched_get_priority_min(SCHED_IDLE), _ret == 0); #endif } -END_TEST() \ No newline at end of file +END_TEST() diff --git a/test/src/apps/sched/sched_attr_idle.c b/test/src/apps/sched/sched_param_idle.c similarity index 99% rename from test/src/apps/sched/sched_attr_idle.c rename to test/src/apps/sched/sched_param_idle.c index cd20d6da2..435c747f7 100644 --- a/test/src/apps/sched/sched_attr_idle.c +++ b/test/src/apps/sched/sched_param_idle.c @@ -28,4 +28,4 @@ int main() printf("Test completed\n"); return 0; -} \ No newline at end of file +} diff --git a/test/src/apps/scripts/process.sh b/test/src/apps/scripts/process.sh index 44a5dc692..6f387824f 100755 --- a/test/src/apps/scripts/process.sh +++ b/test/src/apps/scripts/process.sh @@ -50,8 +50,9 @@ pthread/pthread_test pty/close_pty pty/open_pty pty/pty_blocking -sched/sched_attr -sched/sched_attr_idle +sched/sched_attr_getset +sched/sched_param_getset +sched/sched_param_idle shm/posix_shm signal_c/kill signal_c/parent_death_signal