diff --git a/kernel/comps/systree/src/attr.rs b/kernel/comps/systree/src/attr.rs index e9dd78ef8..57b12be0c 100644 --- a/kernel/comps/systree/src/attr.rs +++ b/kernel/comps/systree/src/attr.rs @@ -3,29 +3,8 @@ use alloc::collections::BTreeMap; use core::fmt::Debug; -use bitflags::bitflags; - use super::{Error, Result, SysStr}; - -bitflags! { - /// Flags defining the properties and permissions of a `SysAttr`. - #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] - pub struct SysAttrFlags: u32 { - /// Indicates whether the attribute can be read. - const CAN_READ = 1 << 0; - /// Indicates whether the attribute can be written to. - const CAN_WRITE = 1 << 1; - /// Indicates whether an attribute is a binary one - /// (rather than a textual one). - const IS_BINARY = 1 << 4; - } -} - -impl Default for SysAttrFlags { - fn default() -> Self { - Self::CAN_READ - } -} +use crate::SysPerms; /// An attribute may be fetched or updated via the methods of `SysNode` /// such as `SysNode::read_attr` and `SysNode::write_attr`. @@ -35,14 +14,14 @@ pub struct SysAttr { id: u8, /// The name of the attribute. Used to look up the attribute in a `SysAttrSet`. name: SysStr, - /// Flags defining the behavior and permissions of the attribute. - flags: SysAttrFlags, + /// The initial permissions of the attribute. + perms: SysPerms, } impl SysAttr { /// Creates a new attribute. - pub fn new(id: u8, name: SysStr, flags: SysAttrFlags) -> Self { - Self { id, name, flags } + pub fn new(id: u8, name: SysStr, perms: SysPerms) -> Self { + Self { id, name, perms } } /// Returns the unique ID of the attribute within its set. @@ -55,9 +34,9 @@ impl SysAttr { &self.name } - /// Returns the flags associated with the attribute. - pub fn flags(&self) -> SysAttrFlags { - self.flags + /// Returns the [`SysPerms`] representing the initial permissions of the attribute. + pub fn perms(&self) -> SysPerms { + self.perms } } @@ -123,14 +102,14 @@ impl SysAttrSetBuilder { /// Adds an attribute definition to the builder. /// /// If an attribute with the same name already exists, this is a no-op. - pub fn add(&mut self, name: SysStr, flags: SysAttrFlags) -> &mut Self { + pub fn add(&mut self, name: SysStr, perms: SysPerms) -> &mut Self { if self.attrs.contains_key(&name) { return self; } let id = self.next_id; self.next_id += 1; - let new_attr = SysAttr::new(id, name.clone(), flags); + let new_attr = SysAttr::new(id, name.clone(), perms); self.attrs.insert(name, new_attr); self } diff --git a/kernel/comps/systree/src/lib.rs b/kernel/comps/systree/src/lib.rs index 2fbb8ad5f..6117fde0a 100644 --- a/kernel/comps/systree/src/lib.rs +++ b/kernel/comps/systree/src/lib.rs @@ -35,8 +35,8 @@ use component::{init_component, ComponentInitError}; use spin::Once; pub use self::{ - attr::{SysAttr, SysAttrFlags, SysAttrSet, SysAttrSetBuilder}, - node::{SysBranchNode, SysNode, SysNodeId, SysNodeType, SysObj, SysSymlink}, + attr::{SysAttr, SysAttrSet, SysAttrSetBuilder}, + node::{SysBranchNode, SysNode, SysNodeId, SysNodeType, SysObj, SysPerms, SysSymlink}, tree::SysTree, utils::{SymlinkNodeFields, SysBranchNodeFields, SysNormalNodeFields, SysObjFields}, }; diff --git a/kernel/comps/systree/src/node.rs b/kernel/comps/systree/src/node.rs index cd5680e78..6fc742582 100644 --- a/kernel/comps/systree/src/node.rs +++ b/kernel/comps/systree/src/node.rs @@ -13,6 +13,7 @@ use core::{ sync::atomic::{AtomicU64, Ordering}, }; +use bitflags::bitflags; use ostd::mm::{FallibleVmWrite, VmReader, VmWriter}; use super::{Error, Result, SysAttrSet, SysStr}; @@ -154,7 +155,7 @@ pub trait SysNode: SysObj { /// Shows the string value of an attribute. /// - /// Most attributes are textual, rather binary (see `SysAttrFlags::IS_BINARY`). + /// Most attributes are textual, rather binary. /// So using this `show_attr` method is more convenient than /// the `read_attr` method. fn show_attr(&self, name: &str) -> Result { @@ -169,13 +170,19 @@ pub trait SysNode: SysObj { /// Stores the string value of an attribute. /// - /// Most attributes are textual, rather binary (see `SysAttrFlags::IS_BINARY`). + /// Most attributes are textual, rather binary. /// So using this `store_attr` method is more convenient than /// the `write_attr` method. fn store_attr(&self, name: &str, new_val: &str) -> Result { let mut reader = VmReader::from(new_val.as_bytes()).to_fallible(); self.write_attr(name, &mut reader) } + + /// Returns the initial permissions of a node. + /// + /// The FS layer should take the value returned from this method + /// as this initial permissions for the corresponding inode. + fn perms(&self) -> SysPerms; } /// A trait that abstracts any symlink node in a `SysTree`. @@ -296,3 +303,72 @@ impl Default for SysNodeId { Self::new() } } + +bitflags! { + /// Permissions for a node or an attribute in the `SysTree`. + /// + /// This struct is mainly used to provide the initial permissions for nodes and attributes. + /// + /// The concepts of "owner"/"group"/"others" mentioned here are not explicitly represented in + /// systree. They exist primarily to enable finer-grained permission management at + /// the "view" and "control" parts for users. The definitions of these permissions match that + /// of VFS file permissions bit-wise. + /// + /// Users can provide permission modification functionality through additional abstractions at + /// the upper layers. Correspondingly, it is the users' responsibility to do the permission + /// verification at the "view" and "control" parts. + #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] + pub struct SysPerms: u16 { + // One-bit flags: + /// The read permission for owner. + const OWNER_R = 0o0400; + /// The write permission for owner. + const OWNER_W = 0o0200; + /// The execute/search permission for owner. + const OWNER_X = 0o0100; + /// The read permission for group. + const GROUP_R = 0o0040; + /// The write permission for group. + const GROUP_W = 0o0020; + /// The execute/search permission for group. + const GROUP_X = 0o0010; + /// The read permission for others. + const OTHER_R = 0o0004; + /// The write permission for others. + const OTHER_W = 0o0002; + /// The execute/search permission for others. + const OTHER_X = 0o0001; + + // Common multi-bit flags: + const ALL_R = 0o0444; + const ALL_W = 0o0222; + const ALL_X = 0o0111; + const ALL_RW = 0o0666; + const ALL_RX = 0o0555; + const ALL_RWX = 0o0777; + } +} + +impl SysPerms { + /// Default read-only permissions for nodes (owner/group/others can read+execute) + pub const DEFAULT_RO_PERMS: Self = Self::ALL_RX; + + /// Default read-write permissions for nodes (owner has full, group/others read+execute) + pub const DEFAULT_RW_PERMS: Self = Self::ALL_RX.union(Self::OWNER_W); + + /// Default read-only permissions for attributes (owner/group/others can read) + pub const DEFAULT_RO_ATTR_PERMS: Self = Self::ALL_R; + + /// Default read-write permissions for attributes (owner read+write, group/others read) + pub const DEFAULT_RW_ATTR_PERMS: Self = Self::ALL_R.union(Self::OWNER_W); + + /// Returns whether read operations are allowed. + pub fn can_read(&self) -> bool { + self.intersects(Self::ALL_R) + } + + /// Returns whether write operations are allowed. + pub fn can_write(&self) -> bool { + self.intersects(Self::ALL_W) + } +} diff --git a/kernel/comps/systree/src/test.rs b/kernel/comps/systree/src/test.rs index b8f09aea8..813208838 100644 --- a/kernel/comps/systree/src/test.rs +++ b/kernel/comps/systree/src/test.rs @@ -16,8 +16,8 @@ use ostd::{ use super::{ impl_cast_methods_for_branch, impl_cast_methods_for_symlink, Error, Result, SymlinkNodeFields, - SysAttrFlags, SysAttrSet, SysAttrSetBuilder, SysBranchNode, SysBranchNodeFields, SysNode, - SysNodeId, SysNodeType, SysObj, SysStr, SysSymlink, SysTree, + SysAttrSet, SysAttrSetBuilder, SysBranchNode, SysBranchNodeFields, SysNode, SysNodeId, + SysNodeType, SysObj, SysPerms, SysStr, SysSymlink, SysTree, }; #[derive(Debug)] @@ -29,12 +29,9 @@ impl DeviceNode { fn new(name: SysStr) -> Arc { let mut builder = SysAttrSetBuilder::new(); builder - .add(Cow::Borrowed("model"), SysAttrFlags::CAN_READ) - .add(Cow::Borrowed("vendor"), SysAttrFlags::CAN_READ) - .add( - Cow::Borrowed("status"), - SysAttrFlags::CAN_READ | SysAttrFlags::CAN_WRITE, - ); + .add(Cow::Borrowed("model"), SysPerms::DEFAULT_RO_ATTR_PERMS) + .add(Cow::Borrowed("vendor"), SysPerms::DEFAULT_RO_ATTR_PERMS) + .add(Cow::Borrowed("status"), SysPerms::DEFAULT_RW_ATTR_PERMS); let attrs = builder.build().expect("Failed to build attribute set"); @@ -76,7 +73,7 @@ impl SysNode for DeviceNode { let attr = self.fields.attr_set().get(name).unwrap(); // Check if attribute is readable - if !attr.flags().contains(SysAttrFlags::CAN_READ) { + if !attr.perms().can_read() { return Err(Error::PermissionDenied); } let value = match name { @@ -101,7 +98,7 @@ impl SysNode for DeviceNode { .ok_or(Error::AttributeError)?; // Check if attribute is writable - if !attr.flags().contains(SysAttrFlags::CAN_WRITE) { + if !attr.perms().can_write() { return Err(Error::PermissionDenied); } @@ -114,6 +111,10 @@ impl SysNode for DeviceNode { Ok(read_len) } + + fn perms(&self) -> SysPerms { + SysPerms::DEFAULT_RW_PERMS + } } #[inherit_methods(from = "self.fields")] diff --git a/kernel/comps/systree/src/tree.rs b/kernel/comps/systree/src/tree.rs index add2e8992..77ff21da8 100644 --- a/kernel/comps/systree/src/tree.rs +++ b/kernel/comps/systree/src/tree.rs @@ -12,7 +12,7 @@ use super::{ node::{SysBranchNode, SysNode, SysNodeId, SysNodeType, SysObj}, Error, Result, SysStr, }; -use crate::{impl_cast_methods_for_branch, SysBranchNodeFields}; +use crate::{impl_cast_methods_for_branch, SysBranchNodeFields, SysPerms}; #[derive(Debug)] pub struct SysTree { @@ -88,6 +88,10 @@ impl SysNode for RootNode { fn write_attr(&self, _name: &str, _reader: &mut VmReader) -> Result { Err(Error::AttributeError) } + + fn perms(&self) -> SysPerms { + SysPerms::DEFAULT_RO_PERMS + } } #[inherit_methods(from = "self.fields")] diff --git a/kernel/src/fs/sysfs/test.rs b/kernel/src/fs/sysfs/test.rs index b2c94a036..fd1734f0b 100644 --- a/kernel/src/fs/sysfs/test.rs +++ b/kernel/src/fs/sysfs/test.rs @@ -14,9 +14,8 @@ use core::fmt::Debug; use aster_systree::{ impl_cast_methods_for_branch, impl_cast_methods_for_node, impl_cast_methods_for_symlink, init_for_ktest, singleton as systree_singleton, Error as SysTreeError, Result as SysTreeResult, - SymlinkNodeFields, SysAttrFlags, SysAttrSet, SysAttrSetBuilder, SysBranchNode, - SysBranchNodeFields, SysNode, SysNodeId, SysNodeType, SysNormalNodeFields, SysObj, SysStr, - SysSymlink, SysTree, + SymlinkNodeFields, SysAttrSet, SysAttrSetBuilder, SysBranchNode, SysBranchNodeFields, SysNode, + SysNodeId, SysNodeType, SysNormalNodeFields, SysObj, SysPerms, SysStr, SysSymlink, SysTree, }; use inherit_methods_macro::inherit_methods; use ostd::{ @@ -50,13 +49,16 @@ impl MockLeafNode { let mut builder = SysAttrSetBuilder::new(); let mut data = BTreeMap::new(); for &attr_name in read_attrs { - builder.add(Cow::Owned(attr_name.to_string()), SysAttrFlags::CAN_READ); + builder.add( + Cow::Owned(attr_name.to_string()), + SysPerms::DEFAULT_RO_ATTR_PERMS, + ); data.insert(attr_name.to_string(), format!("val_{}", attr_name)); // Initial value } for &attr_name in write_attrs { builder.add( Cow::Owned(attr_name.to_string()), - SysAttrFlags::CAN_READ | SysAttrFlags::CAN_WRITE, + SysPerms::DEFAULT_RW_ATTR_PERMS, ); data.insert(attr_name.to_string(), format!("val_{}", attr_name)); // Initial value } @@ -97,7 +99,7 @@ impl SysNode for MockLeafNode { .attr_set() .get(name) .ok_or(SysTreeError::AttributeError)?; - if !attr.flags().contains(SysAttrFlags::CAN_READ) { + if !attr.perms().can_read() { return Err(SysTreeError::PermissionDenied); } let data = self.data.read(); @@ -114,7 +116,7 @@ impl SysNode for MockLeafNode { .attr_set() .get(name) .ok_or(SysTreeError::AttributeError)?; - if !attr.flags().contains(SysAttrFlags::CAN_WRITE) { + if !attr.perms().can_write() { return Err(SysTreeError::PermissionDenied); } @@ -131,6 +133,10 @@ impl SysNode for MockLeafNode { Ok(read_len) } + + fn perms(&self) -> SysPerms { + SysPerms::DEFAULT_RW_PERMS + } } // Refactor MockBranchNode to use SysBranchNodeFields @@ -144,7 +150,10 @@ impl MockBranchNode { let name_owned: SysStr = name.to_string().into(); // Convert to owned SysStr let mut builder = SysAttrSetBuilder::new(); - builder.add(Cow::Borrowed("branch_attr"), SysAttrFlags::CAN_READ); + builder.add( + Cow::Borrowed("branch_attr"), + SysPerms::DEFAULT_RO_ATTR_PERMS, + ); let attrs = builder .build() .expect("Failed to build branch attribute set"); @@ -184,7 +193,7 @@ impl SysNode for MockBranchNode { .attr_set() .get(name) .ok_or(SysTreeError::AttributeError)?; - if !attr.flags().contains(SysAttrFlags::CAN_READ) { + if !attr.perms().can_read() { return Err(SysTreeError::PermissionDenied); } let value = match name { @@ -203,12 +212,16 @@ impl SysNode for MockBranchNode { .attr_set() .get(name) .ok_or(SysTreeError::AttributeError)?; - if !attr.flags().contains(SysAttrFlags::CAN_WRITE) { + if !attr.perms().can_write() { return Err(SysTreeError::PermissionDenied); } // No writable attrs in this mock for now Err(SysTreeError::AttributeError) } + + fn perms(&self) -> SysPerms { + SysPerms::DEFAULT_RW_PERMS + } } #[inherit_methods(from = "self.fields")] @@ -432,7 +445,7 @@ fn test_sysfs_write_attr() { // Verification: Write to the sysfs files and check if the operation // is correctly delegated to the underlying mock systree node's write_attr method, - // respecting read/write permissions derived from SysAttrFlags. + // respecting read/write permissions derived from SysPerms. // Write to rw_attr1 let new_val = "new_value"; @@ -554,17 +567,17 @@ fn test_sysfs_mode_permissions() { let rw_attr_inode = leaf1_dir_inode.lookup("rw_attr1").unwrap(); // Sysfs file for read-write attr // Verification: Check that the default mode (permissions) of the sysfs files/dirs - // correctly reflects the SysAttrFlags of the underlying systree attributes/nodes. + // correctly reflects the SysPerms of the underlying systree attributes/nodes. // Also test that set_mode works on the sysfs inode. - // Check default modes based on SysAttrFlags + // Check default modes based on SysPerms let r_mode = r_attr_inode.mode().unwrap(); assert!(r_mode.contains(InodeMode::S_IRUSR | InodeMode::S_IRGRP | InodeMode::S_IROTH)); // 0o444 - assert!(!r_mode.contains(InodeMode::S_IWUSR | InodeMode::S_IWGRP | InodeMode::S_IWOTH)); // Not 0o222 + assert!(!r_mode.contains(InodeMode::S_IWUSR)); // Not 0o200 let rw_mode = rw_attr_inode.mode().unwrap(); assert!(rw_mode.contains(InodeMode::S_IRUSR | InodeMode::S_IRGRP | InodeMode::S_IROTH)); // 0o444 - assert!(rw_mode.contains(InodeMode::S_IWUSR | InodeMode::S_IWGRP | InodeMode::S_IWOTH)); // 0o222 + assert!(rw_mode.contains(InodeMode::S_IWUSR)); // 0o200 // Test set_mode let new_mode = InodeMode::from_bits_truncate(0o600); // rw------- diff --git a/kernel/src/fs/utils/inode.rs b/kernel/src/fs/utils/inode.rs index eaba08626..11ae38f8c 100644 --- a/kernel/src/fs/utils/inode.rs +++ b/kernel/src/fs/utils/inode.rs @@ -5,6 +5,7 @@ use core::{any::TypeId, time::Duration}; use aster_rights::Full; +use aster_systree::SysPerms; use core2::io::{Error as IoError, ErrorKind as IoErrorKind, Result as IoResult, Write}; use ostd::task::Task; @@ -200,6 +201,12 @@ impl InodeMode { } } +impl From for InodeMode { + fn from(value: SysPerms) -> Self { + InodeMode::from_bits_truncate(value.bits()) + } +} + #[derive(Debug, Clone, Copy)] pub struct Metadata { pub dev: u64,