Skip to content
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

Refactors regmgr_call and privileges #663

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/kernel/src/fs/ioctl.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::errno::ENOTTY;
use crate::syscalls::SysArg;
use crate::syscalls::SysErr;
use std::convert::Into;

/// This macro does some compile time verification to ensure we don't mistype anything.
/// It also ensures that we don't miss any commands, since [`IoCmd::try_from_raw_parts`] will panic with a todo! if it encounters an unknown command.
Expand Down Expand Up @@ -43,15 +45,17 @@ macro_rules! commands {
pub const IOC_OUT: u32 = 0x40000000;
pub const IOC_IN: u32 = 0x80000000;

pub fn try_from_raw_parts(cmd: u64, arg: *mut u8) -> Result<Self, SysErr> {
/// # Safety
/// `arg` has to be a pointer to the correct value
pub unsafe fn try_from_raw_parts(cmd: u64, arg: SysArg) -> Result<Self, SysErr> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is even dangerous than the pointer because it hide the fact that the returned value have a lifetime bind to some pointer.

I guess we should required every PRs that use unsafe to be reviewed by me (or someone else who can catch this kind of bug) so we don't have a dangerous bug get merged. Here is an example of what going to happen if we violate the Rust assumption due to unsafe: #48

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried something, take a look

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's that simple? :D

Copy link
Contributor Author

@SuchAFuriousDeath SuchAFuriousDeath Feb 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait a minute. To be completely honest, sometimes I don't fully understand lifetimes. The CStr struct isn't lifetimed (which is why from_ptr has a lifetime parameter and returns a lifetimed reference), but our IoCmd and RegMgrCommand are. Doesn't that make any difference?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point is try_from_raw_parts need to have an explicit lifetime and document it how it bind to the pointer.

let cmd = cmd as u32;

if Self::is_invalid(cmd) {
return Err(SysErr::Raw(ENOTTY));
}

let cmd = match cmd {
$( $value => Self::$variant $( ( unsafe { &mut *(arg as *mut $type) } ) )? ,)*
$( $value => Self::$variant $( ( unsafe { &mut *(Into::<*mut $type>::into(arg)) } ) )? ,)*
_ => todo!("Unhandled ioctl command {:#x}", cmd)
};

Expand Down
2 changes: 1 addition & 1 deletion src/kernel/src/fs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ impl Fs {
fn sys_ioctl(self: &Arc<Self>, td: &VThread, i: &SysIn) -> Result<SysOut, SysErr> {
let fd: i32 = i.args[0].try_into().unwrap();
// Our IoCmd contains both the command and the argument (if there is one).
let cmd = IoCmd::try_from_raw_parts(i.args[1].into(), i.args[2].into())?;
let cmd = unsafe { IoCmd::try_from_raw_parts(i.args[1].into(), i.args[2].into())? };

info!("Executing ioctl({cmd:?}) on file descriptor {fd}.");

Expand Down
38 changes: 38 additions & 0 deletions src/kernel/src/regmgr/command.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
use super::RegError;
use crate::syscalls::SysArg;
use std::convert::Into;

#[repr(u32)]
pub(super) enum RegMgrCommand<'a> {
SetInt(&'a SetIntArg) = 0x18,
Unk1(&'a Unk1Arg) = 0x19,
}
impl<'a> RegMgrCommand<'a> {
/// # Safety
/// `arg` has to be a pointer to the correct value
pub unsafe fn try_from_raw_parts(cmd: u32, arg: SysArg) -> Result<Self, RegError> {
match cmd {
0x18 => Ok(RegMgrCommand::SetInt(unsafe {
&*(Into::<*mut _>::into(arg))
})),
0x19 => Ok(RegMgrCommand::Unk1(unsafe {
&*(Into::<*mut _>::into(arg))
})),
0x27 | 0x40.. => Err(RegError::V800d0219),
v => todo!("RegMgrCommand({v})"),
}
}
}

#[repr(C)]
pub(super) struct SetIntArg {
pub v1: u64,
pub v2: u32,
pub value: i32,
}

#[repr(C)]
pub(super) struct Unk1Arg {
pub v1: u64,
pub v2: u32,
}
44 changes: 28 additions & 16 deletions src/kernel/src/regmgr/mod.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
pub use self::key::*;

use self::command::*;
use crate::errno::EINVAL;
use crate::process::VThread;
use crate::syscalls::{SysErr, SysIn, SysOut, Syscalls};
use crate::ucred::Ucred;
use crate::ucred::{Privilege, Ucred};
use crate::{info, warn};
use std::fmt::{Display, Formatter};
use std::num::NonZeroI32;
Expand All @@ -12,6 +11,9 @@ use std::ptr::read;
use std::sync::Arc;
use thiserror::Error;

pub use self::key::*;

mod command;
mod key;

/// An implementation of PS4 registry manager.
Expand All @@ -31,28 +33,40 @@ impl RegMgr {
// Get arguments.
let op: u32 = i.args[0].try_into().unwrap();
let buf: *mut i32 = i.args[2].into();
let req: *const u8 = i.args[3].into();
let req = i.args[3];
let reqlen: usize = i.args[4].into();

// TODO: Check the result of priv_check(td, 682).
let td = VThread::current().unwrap();

td.cred().priv_check(Privilege::SCE682)?;

if buf.is_null() {
todo!("regmgr_call with buf = null");
}

if req.is_null() {
if req == 0 {
todo!("regmgr_call with req = null");
}

if reqlen > 2048 {
todo!("regmgr_call with reqlen > 2048");
}

let command = match unsafe { RegMgrCommand::try_from_raw_parts(op, req) } {
Ok(v) => v,
Err(e) => {
warn!(e, "regmgr_call({op}) failed");
unsafe { *buf = e.code() };
return Ok(SysOut::ZERO);
}
};

// Execute the operation.
let r = match op {
0x18 => {
let v1 = unsafe { read::<u64>(req as _) };
let v2 = unsafe { read::<u32>(req.add(8) as _) };
let value = unsafe { read::<i32>(req.add(12) as _) };
let r = match command {
RegMgrCommand::SetInt(arg) => {
let v1 = arg.v1;
let v2 = arg.v2;
let value = arg.value;

info!(
"Attempting to set registry with v1: {}, v2: {}, value: {}.",
Expand All @@ -64,15 +78,13 @@ impl RegMgr {
self.set_int(k, value)
})
}
0x19 => {
let v1 = unsafe { read::<u64>(req as _) };
let v2 = unsafe { read::<u32>(req.add(8) as _) };
RegMgrCommand::Unk1(arg) => {
let v1 = arg.v1;
let v2 = arg.v2;

self.decode_key(v1, v2, td.cred(), 1)
.and_then(|k| todo!("regmgr_call({op}) with matched key = {k}"))
}
0x27 | 0x40.. => Err(RegError::V800d0219),
v => todo!("regmgr_call({v})"),
};

// Write the result.
Expand Down
1 change: 1 addition & 0 deletions src/kernel/src/ucred/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ impl Ucred {
Privilege::MAXFILES
| Privilege::PROC_SETLOGIN
| Privilege::SCE680
| Privilege::SCE682
| Privilege::SCE683
| Privilege::SCE686 => self.is_system(),
v => todo!("priv_check_cred(cred, {v})"),
Expand Down
56 changes: 30 additions & 26 deletions src/kernel/src/ucred/privilege.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,33 +8,37 @@ use std::fmt::{Display, Formatter};
#[derive(Clone, Copy, PartialEq, Eq)]
pub struct Privilege(i32);

impl Privilege {
pub const MAXFILES: Self = Self(3);
pub const PROC_SETLOGIN: Self = Self(161);
pub const VFS_READ: Self = Self(310);
pub const VFS_WRITE: Self = Self(311);
pub const VFS_ADMIN: Self = Self(312);
pub const VFS_EXEC: Self = Self(313);
pub const VFS_LOOKUP: Self = Self(314);
pub const SCE680: Self = Self(680);
pub const SCE683: Self = Self(683);
pub const SCE686: Self = Self(686);
}
macro_rules! privileges {
($($name:ident($value:expr)),*) => {
impl Privilege {
$(
pub const $name: Self = Self($value);
)*
}

impl Display for Privilege {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match *self {
Self::MAXFILES => f.write_str("PRIV_MAXFILES"),
Self::PROC_SETLOGIN => f.write_str("PRIV_PROC_SETLOGIN"),
Self::VFS_READ => f.write_str("PRIV_VFS_READ"),
Self::VFS_WRITE => f.write_str("PRIV_VFS_WRITE"),
Self::VFS_ADMIN => f.write_str("PRIV_VFS_ADMIN"),
Self::VFS_EXEC => f.write_str("PRIV_VFS_EXEC"),
Self::VFS_LOOKUP => f.write_str("PRIV_VFS_LOOKUP"),
Self::SCE680 => f.write_str("SCE680"),
Self::SCE683 => f.write_str("SCE683"),
Self::SCE686 => f.write_str("SCE686"),
v => v.0.fmt(f),
impl Display for Privilege {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match *self {
$(
Self($value) => f.write_str(stringify!($name)),
)*
v => v.0.fmt(f),
}
}
}
}
}

privileges! {
MAXFILES(3),
PROC_SETLOGIN(161),
VFS_READ(310),
VFS_WRITE(311),
VFS_ADMIN(312),
VFS_EXEC(313),
VFS_LOOKUP(314),
SCE680(680),
SCE682(682),
SCE683(683),
SCE686(686)
}