Fix a potential deadlock when dereference procfs's symlink

This commit is contained in:
LI Qing 2021-06-01 16:18:58 +08:00 committed by Zongmin.Gu
parent dd12fca9a1
commit 69c79d8252
22 changed files with 48 additions and 27 deletions

@ -50,7 +50,7 @@ pub fn do_faccessat(
let inode = {
let path = fs_path.to_abs_path()?;
let current = current!();
let fs = current.fs().lock().unwrap();
let fs = current.fs().read().unwrap();
if flags.contains(AccessibilityCheckFlags::AT_SYMLINK_NOFOLLOW) {
fs.lookup_inode_no_follow(&path)?
} else {

@ -61,7 +61,7 @@ pub fn do_fchmodat(fs_path: &FsPath, mode: FileMode) -> Result<()> {
let inode = {
let path = fs_path.to_abs_path()?;
let current = current!();
let fs = current.fs().lock().unwrap();
let fs = current.fs().read().unwrap();
fs.lookup_inode(&path)?
};
let mut info = inode.metadata()?;

@ -16,7 +16,7 @@ pub fn do_fchownat(fs_path: &FsPath, uid: u32, gid: u32, flags: ChownFlags) -> R
let inode = {
let path = fs_path.to_abs_path()?;
let current = current!();
let fs = current.fs().lock().unwrap();
let fs = current.fs().read().unwrap();
if flags.contains(ChownFlags::AT_SYMLINK_NOFOLLOW) {
fs.lookup_inode_no_follow(&path)?
} else {

@ -74,12 +74,12 @@ impl<'a> FsPath<'a> {
FsPathInner::Fd(fd) => get_abs_path_by_fd(*fd)?,
FsPathInner::CwdRelative(path) => {
let current = current!();
let fs = current.fs().lock().unwrap();
let fs = current.fs().read().unwrap();
fs.convert_to_abs_path(path)
}
FsPathInner::Cwd => {
let current = current!();
let fs = current.fs().lock().unwrap();
let fs = current.fs().read().unwrap();
fs.cwd().to_owned()
}
};

@ -18,7 +18,7 @@ pub fn do_linkat(old_fs_path: &FsPath, new_fs_path: &FsPath, flags: LinkFlags) -
let (inode, new_dir_inode) = {
let oldpath = old_fs_path.to_abs_path()?;
let current = current!();
let fs = current.fs().lock().unwrap();
let fs = current.fs().read().unwrap();
let inode = if flags.contains(LinkFlags::AT_SYMLINK_FOLLOW) {
fs.lookup_inode(&oldpath)?
} else {

@ -7,7 +7,7 @@ pub fn do_mkdirat(fs_path: &FsPath, mode: usize) -> Result<()> {
let (dir_path, file_name) = split_path(&path);
let inode = {
let current = current!();
let fs = current.fs().lock().unwrap();
let fs = current.fs().read().unwrap();
fs.lookup_inode(dir_path)?
};
if inode.find(file_name).is_ok() {

@ -8,7 +8,7 @@ pub fn do_openat(fs_path: &FsPath, flags: u32, mode: u32) -> Result<FileDesc> {
let path = fs_path.to_abs_path()?;
let current = current!();
let fs = current.fs().lock().unwrap();
let fs = current.fs().read().unwrap();
let file_ref: Arc<dyn File> = fs.open_file(&path, flags, mode)?;

@ -9,7 +9,7 @@ pub fn do_renameat(old_fs_path: &FsPath, new_fs_path: &FsPath) -> Result<()> {
let oldpath = old_fs_path.to_abs_path()?;
let newpath = new_fs_path.to_abs_path()?;
let current = current!();
let fs = current.fs().lock().unwrap();
let fs = current.fs().read().unwrap();
let (old_dir_path, old_file_name) = split_path(&oldpath);
let (new_dir_path, new_file_name) = split_path(&newpath);

@ -6,7 +6,7 @@ pub fn do_rmdir(path: &str) -> Result<()> {
let (dir_path, file_name) = split_path(&path);
let dir_inode = {
let current = current!();
let fs = current.fs().lock().unwrap();
let fs = current.fs().read().unwrap();
fs.lookup_inode(dir_path)?
};
let file_inode = dir_inode.find(file_name)?;

@ -147,7 +147,7 @@ pub fn do_fstatat(fs_path: &FsPath, flags: StatFlags) -> Result<Stat> {
let inode = {
let path = fs_path.to_abs_path()?;
let current = current!();
let fs = current.fs().lock().unwrap();
let fs = current.fs().read().unwrap();
if flags.contains(StatFlags::AT_SYMLINK_NOFOLLOW) {
fs.lookup_inode_no_follow(&path)?
} else {

@ -7,7 +7,7 @@ pub fn do_readlinkat(fs_path: &FsPath, buf: &mut [u8]) -> Result<usize> {
let file_path = {
let inode = {
let current = current!();
let fs = current.fs().lock().unwrap();
let fs = current.fs().read().unwrap();
fs.lookup_inode_no_follow(&path)?
};
if inode.metadata()?.type_ != FileType::SymLink {
@ -38,7 +38,7 @@ pub fn do_symlinkat(target: &str, link_path: &FsPath) -> Result<usize> {
let (dir_path, link_name) = split_path(&link_path);
let dir_inode = {
let current = current!();
let fs = current.fs().lock().unwrap();
let fs = current.fs().read().unwrap();
fs.lookup_inode(dir_path)?
};
if !dir_inode.allow_write()? {

@ -4,7 +4,7 @@ pub fn do_truncate(path: &str, len: usize) -> Result<()> {
debug!("truncate: path: {:?}, len: {}", path, len);
let inode = {
let current = current!();
let fs = current.fs().lock().unwrap();
let fs = current.fs().read().unwrap();
fs.lookup_inode(&path)?
};
inode.resize(len)?;

@ -10,7 +10,7 @@ fn do_unlink(path: &str) -> Result<()> {
let (dir_path, file_name) = split_path(&path);
let dir_inode = {
let current = current!();
let fs = current.fs().lock().unwrap();
let fs = current.fs().read().unwrap();
fs.lookup_inode(dir_path)?
};
let file_inode = dir_inode.find(file_name)?;

@ -4,14 +4,15 @@ pub fn do_chdir(path: &str) -> Result<()> {
debug!("chdir: path: {:?}", path);
let current = current!();
let mut fs = current.fs().lock().unwrap();
let inode = fs.lookup_inode(path)?;
let inode = {
let fs = current.fs().read().unwrap();
fs.lookup_inode(path)?
};
let info = inode.metadata()?;
if info.type_ != FileType::Dir {
return_errno!(ENOTDIR, "cwd must be directory");
}
fs.set_cwd(path)?;
current.fs().write().unwrap().set_cwd(path)?;
Ok(())
}

@ -3,7 +3,7 @@ use super::*;
pub fn do_getcwd() -> Result<String> {
debug!("getcwd");
let thread = current!();
let fs = thread.fs().lock().unwrap();
let fs = thread.fs().read().unwrap();
let cwd = fs.cwd().to_owned();
Ok(cwd)
}

@ -165,7 +165,7 @@ impl ProcCwdSymINode {
impl ProcINode for ProcCwdSymINode {
fn generate_data_in_bytes(&self) -> vfs::Result<Vec<u8>> {
let main_thread = self.0.main_thread().ok_or(FsError::EntryNotFound)?;
let fs = main_thread.fs().lock().unwrap();
let fs = main_thread.fs().read().unwrap();
Ok(fs.cwd().to_owned().into_bytes())
}
}
@ -181,7 +181,7 @@ impl ProcRootSymINode {
impl ProcINode for ProcRootSymINode {
fn generate_data_in_bytes(&self) -> vfs::Result<Vec<u8>> {
let main_thread = self.0.main_thread().ok_or(FsError::EntryNotFound)?;
let fs = main_thread.fs().lock().unwrap();
let fs = main_thread.fs().read().unwrap();
Ok(fs.root().to_owned().into_bytes())
}
}

@ -126,7 +126,7 @@ impl UnixPath {
None
} else {
let thread = current!();
let fs = thread.fs().lock().unwrap();
let fs = thread.fs().read().unwrap();
let cwd = fs.cwd().to_owned();
Some(cwd)

@ -75,7 +75,7 @@ pub fn load_file_hdr_to_vec(
) -> Result<(Arc<dyn INode>, Vec<u8>, Option<ElfHeader>)> {
let inode = current_ref
.fs()
.lock()
.read()
.unwrap()
.lookup_inode(file_path)
.map_err(|e| errno!(e.errno(), "cannot find the file"))?;

@ -185,7 +185,7 @@ fn new_process(
let files = init_files(current_ref, file_actions, host_stdio_fds)?;
Arc::new(SgxMutex::new(files))
};
let fs_ref = Arc::new(SgxMutex::new(current_ref.fs().lock().unwrap().clone()));
let fs_ref = Arc::new(RwLock::new(current_ref.fs().read().unwrap().clone()));
let sched_ref = Arc::new(SgxMutex::new(current_ref.sched().lock().unwrap().clone()));
let rlimit_ref = Arc::new(SgxMutex::new(current_ref.rlimits().lock().unwrap().clone()));
@ -254,7 +254,7 @@ fn init_files(
let file_ref =
current_ref
.fs()
.lock()
.read()
.unwrap()
.open_file(path.as_str(), oflag, mode)?;
let creation_flags = CreationFlags::from_bits_truncate(oflag);

@ -62,6 +62,6 @@ pub type ProcessRef = Arc<Process>;
pub type ThreadRef = Arc<Thread>;
pub type FileTableRef = Arc<SgxMutex<FileTable>>;
pub type ProcessVMRef = Arc<ProcessVM>;
pub type FsViewRef = Arc<SgxMutex<FsView>>;
pub type FsViewRef = Arc<RwLock<FsView>>;
pub type SchedAgentRef = Arc<SgxMutex<SchedAgent>>;
pub type ResourceLimitsRef = Arc<SgxMutex<ResourceLimits>>;

@ -106,6 +106,12 @@ unsafe impl<#[may_dangle] T: ?Sized> Drop for RwLock<T> {
}
}
impl<T: ?Sized + Default> Default for RwLock<T> {
fn default() -> RwLock<T> {
RwLock::new(Default::default())
}
}
impl<T: core::fmt::Debug> fmt::Debug for RwLock<T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("RwLock")

@ -76,6 +76,19 @@ static int test_readlink_from_proc_self_root() {
return 0;
}
static int test_create_and_unlink_file_from_proc_self_root() {
const char *proc_root_file = "/proc/self/root/test_file";
int fd = open(proc_root_file, O_RDONLY | O_CREAT | O_TRUNC, 00666);
if (fd < 0) {
THROW_ERROR("failed to create a file");
}
close(fd);
if (unlink(proc_root_file) < 0) {
THROW_ERROR("failed to unlink the created file");
}
return 0;
}
static int test_read_from_proc_self_cmdline() {
char absolute_path[PATH_MAX] = { 0 };
const char *proc_cmdline = "/proc/self/cmdline";
@ -137,6 +150,7 @@ static test_case_t test_cases[] = {
TEST_CASE(test_readlink_from_proc_self_exe),
TEST_CASE(test_readlink_from_proc_self_cwd),
TEST_CASE(test_readlink_from_proc_self_root),
TEST_CASE(test_create_and_unlink_file_from_proc_self_root),
TEST_CASE(test_read_from_proc_self_cmdline),
TEST_CASE(test_read_from_proc_meminfo),
TEST_CASE(test_read_from_proc_cpuinfo),