fd_server: use RwLock instead of Mutex
`nix::dir::Dir` isn't Sync (required by `RwLock`), so I had to replace
it with `OwnedFd`.
Bug: 220386264
Test: atest AuthFsHostTest
Test: performance change before vs after is hard to tell
Change-Id: Ib3fd9047970a856b532234f0bebad652172e12de
diff --git a/authfs/fd_server/src/aidl.rs b/authfs/fd_server/src/aidl.rs
index 2975aa4..3a3cdb2 100644
--- a/authfs/fd_server/src/aidl.rs
+++ b/authfs/fd_server/src/aidl.rs
@@ -17,7 +17,7 @@
use anyhow::Result;
use log::error;
use nix::{
- dir::Dir, errno::Errno, fcntl::openat, fcntl::OFlag, sys::stat::fchmod, sys::stat::mkdirat,
+ errno::Errno, fcntl::openat, fcntl::OFlag, sys::stat::fchmod, sys::stat::mkdirat,
sys::stat::mode_t, sys::stat::Mode, sys::statvfs::statvfs, sys::statvfs::Statvfs,
unistd::unlinkat, unistd::UnlinkatFlags,
};
@@ -29,8 +29,9 @@
use std::os::unix::fs::FileExt;
use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
use std::path::{Component, Path, PathBuf, MAIN_SEPARATOR};
-use std::sync::{Arc, Mutex};
+use std::sync::{Arc, RwLock};
+use crate::common::OwnedFd;
use crate::fsverity;
use authfs_aidl_interface::aidl::com::android::virt::fs::IVirtFdService::{
BnVirtFdService, FsStat::FsStat, IVirtFdService, MAX_REQUESTING_DATA,
@@ -63,21 +64,21 @@
ReadWrite(File),
/// A read-only directory to serve by this server.
- InputDir(Dir),
+ InputDir(OwnedFd),
/// A writable directory to serve by this server.
- OutputDir(Dir),
+ OutputDir(OwnedFd),
}
pub struct FdService {
/// A pool of opened files and directories, which can be looked up by the FD number.
- fd_pool: Arc<Mutex<BTreeMap<i32, FdConfig>>>,
+ fd_pool: Arc<RwLock<BTreeMap<i32, FdConfig>>>,
}
impl FdService {
pub fn new_binder(fd_pool: BTreeMap<i32, FdConfig>) -> Strong<dyn IVirtFdService> {
BnVirtFdService::new_binder(
- FdService { fd_pool: Arc::new(Mutex::new(fd_pool)) },
+ FdService { fd_pool: Arc::new(RwLock::new(fd_pool)) },
BinderFeatures::default(),
)
}
@@ -88,7 +89,7 @@
where
F: FnOnce(&FdConfig) -> BinderResult<R>,
{
- let fd_pool = self.fd_pool.lock().unwrap();
+ let fd_pool = self.fd_pool.read().unwrap();
let fd_config = fd_pool.get(&id).ok_or_else(|| new_errno_error(Errno::EBADF))?;
handle_fn(fd_config)
}
@@ -99,7 +100,7 @@
where
F: FnOnce(&mut FdConfig) -> BinderResult<(i32, FdConfig)>,
{
- let mut fd_pool = self.fd_pool.lock().unwrap();
+ let mut fd_pool = self.fd_pool.write().unwrap();
let fd_config = fd_pool.get_mut(&fd).ok_or_else(|| new_errno_error(Errno::EBADF))?;
let (new_fd, new_fd_config) = create_fn(fd_config)?;
if let btree_map::Entry::Vacant(entry) = fd_pool.entry(new_fd) {
@@ -319,14 +320,12 @@
FdConfig::OutputDir(_) => {
let mode = validate_file_mode(mode)?;
mkdirat(dir_fd, basename, mode).map_err(new_errno_error)?;
- let new_dir = Dir::openat(
- dir_fd,
- basename,
- OFlag::O_DIRECTORY | OFlag::O_RDONLY,
- Mode::empty(),
- )
- .map_err(new_errno_error)?;
- Ok((new_dir.as_raw_fd(), FdConfig::OutputDir(new_dir)))
+ let new_dir_fd =
+ openat(dir_fd, basename, OFlag::O_DIRECTORY | OFlag::O_RDONLY, Mode::empty())
+ .map_err(new_errno_error)?;
+ // SAFETY: new_dir_fd is just created and not an error.
+ let fd_owner = unsafe { OwnedFd::from_raw_fd(new_dir_fd) };
+ Ok((new_dir_fd, FdConfig::OutputDir(fd_owner)))
}
_ => Err(new_errno_error(Errno::ENOTDIR)),
})
@@ -336,7 +335,7 @@
validate_basename(basename)?;
self.handle_fd(dir_fd, |config| match config {
- FdConfig::OutputDir(_dir) => {
+ FdConfig::OutputDir(_) => {
unlinkat(Some(dir_fd), basename, UnlinkatFlags::NoRemoveDir)
.map_err(new_errno_error)?;
Ok(())
@@ -350,7 +349,7 @@
validate_basename(basename)?;
self.handle_fd(dir_fd, |config| match config {
- FdConfig::OutputDir(_dir) => {
+ FdConfig::OutputDir(_) => {
unlinkat(Some(dir_fd), basename, UnlinkatFlags::RemoveDir)
.map_err(new_errno_error)?;
Ok(())
diff --git a/authfs/fd_server/src/common.rs b/authfs/fd_server/src/common.rs
new file mode 100644
index 0000000..f836bac
--- /dev/null
+++ b/authfs/fd_server/src/common.rs
@@ -0,0 +1,35 @@
+/*
+ * Copyright (C) 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+use std::fs::File;
+use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
+
+// TODO: Remove if/when std::os::unix::io::OwnedFd is standardized.
+pub struct OwnedFd {
+ owner: File,
+}
+
+impl FromRawFd for OwnedFd {
+ unsafe fn from_raw_fd(fd: RawFd) -> Self {
+ OwnedFd { owner: File::from_raw_fd(fd) }
+ }
+}
+
+impl AsRawFd for OwnedFd {
+ fn as_raw_fd(&self) -> RawFd {
+ self.owner.as_raw_fd()
+ }
+}
diff --git a/authfs/fd_server/src/main.rs b/authfs/fd_server/src/main.rs
index fe0475f..a1d09fc 100644
--- a/authfs/fd_server/src/main.rs
+++ b/authfs/fd_server/src/main.rs
@@ -23,12 +23,13 @@
//! client can then request the content of file 9 by offset and size.
mod aidl;
+mod common;
mod fsverity;
use anyhow::{bail, Result};
use binder_common::rpc_server::run_rpc_server;
use log::debug;
-use nix::{dir::Dir, sys::stat::umask, sys::stat::Mode};
+use nix::sys::stat::{umask, Mode};
use std::collections::BTreeMap;
use std::fs::File;
use std::os::unix::io::FromRawFd;
@@ -44,12 +45,12 @@
retval >= 0
}
-fn fd_to_file(fd: i32) -> Result<File> {
+fn fd_to_owned<T: FromRawFd>(fd: i32) -> Result<T> {
if !is_fd_valid(fd) {
bail!("Bad FD: {}", fd);
}
// SAFETY: The caller is supposed to provide valid FDs to this process.
- Ok(unsafe { File::from_raw_fd(fd) })
+ Ok(unsafe { T::from_raw_fd(fd) })
}
fn parse_arg_ro_fds(arg: &str) -> Result<(i32, FdConfig)> {
@@ -61,11 +62,11 @@
Ok((
fds[0],
FdConfig::Readonly {
- file: fd_to_file(fds[0])?,
+ file: fd_to_owned(fds[0])?,
// Alternative metadata source, if provided
alt_metadata: fds
.get(1)
- .map(|fd| fd_to_file(*fd))
+ .map(|fd| fd_to_owned(*fd))
.transpose()?
.and_then(|f| parse_fsverity_metadata(f).ok()),
},
@@ -74,7 +75,7 @@
fn parse_arg_rw_fds(arg: &str) -> Result<(i32, FdConfig)> {
let fd = arg.parse::<i32>()?;
- let file = fd_to_file(fd)?;
+ let file = fd_to_owned::<File>(fd)?;
if file.metadata()?.len() > 0 {
bail!("File is expected to be empty");
}
@@ -83,12 +84,12 @@
fn parse_arg_ro_dirs(arg: &str) -> Result<(i32, FdConfig)> {
let fd = arg.parse::<i32>()?;
- Ok((fd, FdConfig::InputDir(Dir::from_fd(fd)?)))
+ Ok((fd, FdConfig::InputDir(fd_to_owned(fd)?)))
}
fn parse_arg_rw_dirs(arg: &str) -> Result<(i32, FdConfig)> {
let fd = arg.parse::<i32>()?;
- Ok((fd, FdConfig::OutputDir(Dir::from_fd(fd)?)))
+ Ok((fd, FdConfig::OutputDir(fd_to_owned(fd)?)))
}
struct Args {
@@ -147,7 +148,7 @@
}
let ready_fd = if let Some(arg) = matches.value_of("ready-fd") {
let fd = arg.parse::<i32>()?;
- Some(fd_to_file(fd)?)
+ Some(fd_to_owned(fd)?)
} else {
None
};