pa-fazil
/
android_packages_modules_Virtualization
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge changes Ib3fd9047,I3126932f,I3450f496 

* changes:
  fd_server: use RwLock instead of Mutex
  authfs: use RwLock instead of Mutex
  Only read backing file if chunk index is in range
This commit is contained in:
Victor Hsieh 2022-03-08 16:36:32 +00:00 committed by Gerrit Code Review
parent 8a118270e6 bf944d734b
commit 03ef64b482
5 changed files with 82 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
authfs/fd_server/src/aidl.rs
View File

@ -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::io;
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 @@ pub enum FdConfig {
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 @@ impl FdService {
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 @@ impl FdService {
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 @@ impl IVirtFdService for FdService {
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 @@ impl IVirtFdService for FdService {
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 @@ impl IVirtFdService for FdService {
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(())

35
authfs/fd_server/src/common.rs Normal file
View File

@ -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()
}
}

19
authfs/fd_server/src/main.rs
View File

@ -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 @@ fn is_fd_valid(fd: i32) -> bool {
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 @@ fn parse_arg_ro_fds(arg: &str) -> Result<(i32, FdConfig)> {
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_ro_fds(arg: &str) -> Result<(i32, FdConfig)> {
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_rw_fds(arg: &str) -> Result<(i32, FdConfig)> {
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 @@ fn parse_args() -> Result<Args> {
}
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
};

17
authfs/src/fsverity/editor.rs
View File

@ -52,7 +52,6 @@
//! Rollback attack is another possible attack, but can be addressed with a rollback counter when
//! possible.
use log::warn;
use std::io;
use std::sync::{Arc, RwLock};
@ -114,26 +113,18 @@ impl<F: ReadByChunk + RandomWrite> VerifiedFileEditor<F> {
buf: &mut ChunkBuffer,
merkle_tree_locked: &MerkleLeaves,
) -> io::Result<usize> {
let size = self.read_backing_chunk_unverified(chunk_index, buf)?;
// Ensure the returned buffer matches the known hash.
debug_assert_usize_is_u64();
if merkle_tree_locked.is_index_valid(chunk_index as usize) {
let size = self.read_backing_chunk_unverified(chunk_index, buf)?;
// Ensure the returned buffer matches the known hash.
let hash = Sha256Hasher::new()?.update(buf)?.finalize()?;
if !merkle_tree_locked.is_consistent(chunk_index as usize, &hash) {
return Err(io::Error::new(io::ErrorKind::InvalidData, "Inconsistent hash"));
}
Ok(size)
} else {
if size != 0 {
// This is unexpected. For any reason that the file is changed and doesn't match
// the known state, ignore it at the moment. We can still generate correct
// fs-verity digest for an output file.
warn!(
"Ignoring the received {} bytes for index {} beyond the known file size",
size, chunk_index,
);
}
Ok(0)
}
}

32
authfs/src/fusefs.rs
View File

@ -33,7 +33,7 @@ use std::option::Option;
use std::os::unix::ffi::OsStrExt;
use std::path::{Component, Path, PathBuf};
use std::sync::atomic::{AtomicU64, Ordering};
use std::sync::{Arc, Mutex};
use std::sync::{Arc, RwLock};
use std::time::Duration;
use crate::common::{divide_roundup, ChunkedSizeIter, CHUNK_SIZE};
@ -188,7 +188,7 @@ type DirHandleTable = BTreeMap<Handle, Arc<DirEntriesSnapshot>>;
pub struct AuthFs {
/// Table for `Inode` to `InodeState` lookup. This needs to be `Sync` to be used in
/// `fuse::worker::start_message_loop`.
inode_table: Mutex<BTreeMap<Inode, InodeState>>,
inode_table: RwLock<BTreeMap<Inode, InodeState>>,
/// The next available inode number.
next_inode: AtomicU64,
@ -200,7 +200,7 @@ pub struct AuthFs {
///
/// Currently, no code locks `dir_handle_table` and `inode_table` at the same time to avoid
/// deadlock.
dir_handle_table: Mutex<DirHandleTable>,
dir_handle_table: RwLock<DirHandleTable>,
/// The next available handle number.
next_handle: AtomicU64,
@ -222,9 +222,9 @@ impl AuthFs {
);
AuthFs {
inode_table: Mutex::new(inode_table),
inode_table: RwLock::new(inode_table),
next_inode: AtomicU64::new(ROOT_INODE + 1),
dir_handle_table: Mutex::new(BTreeMap::new()),
dir_handle_table: RwLock::new(BTreeMap::new()),
next_handle: AtomicU64::new(1),
remote_fs_stats_reader,
}
@ -321,7 +321,7 @@ impl AuthFs {
where
F: FnOnce(&AuthFsEntry) -> io::Result<R>,
{
let inode_table = self.inode_table.lock().unwrap();
let inode_table = self.inode_table.read().unwrap();
handle_inode_locked(&inode_table, inode, |inode_state| handle_fn(&inode_state.entry))
}
@ -343,7 +343,7 @@ impl AuthFs {
where
F: FnOnce(&mut AuthFsEntry, &Path, Inode) -> io::Result<AuthFsEntry>,
{
let mut inode_table = self.inode_table.lock().unwrap();
let mut inode_table = self.inode_table.write().unwrap();
let (new_inode, new_file_entry) = handle_inode_mut_locked(
&mut inode_table,
&parent_inode,
@ -368,7 +368,7 @@ impl AuthFs {
dir_entries: Vec<AuthFsDirEntry>,
) -> io::Result<(Option<Handle>, FuseOpenOptions)> {
let handle = self.next_handle.fetch_add(1, Ordering::Relaxed);
let mut dir_handle_table = self.dir_handle_table.lock().unwrap();
let mut dir_handle_table = self.dir_handle_table.write().unwrap();
if let btree_map::Entry::Vacant(value) = dir_handle_table.entry(handle) {
value.insert(Arc::new(dir_entries));
Ok((Some(handle), FuseOpenOptions::empty()))
@ -511,7 +511,7 @@ impl FileSystem for AuthFs {
}
fn lookup(&self, _ctx: Context, parent: Inode, name: &CStr) -> io::Result<Entry> {
let mut inode_table = self.inode_table.lock().unwrap();
let mut inode_table = self.inode_table.write().unwrap();
// Look up the entry's inode number in parent directory.
let inode =
@ -566,7 +566,7 @@ impl FileSystem for AuthFs {
}
fn forget(&self, _ctx: Context, inode: Self::Inode, count: u64) {
let mut inode_table = self.inode_table.lock().unwrap();
let mut inode_table = self.inode_table.write().unwrap();
let delete_now = handle_inode_mut_locked(
&mut inode_table,
&inode,
@ -764,7 +764,7 @@ impl FileSystem for AuthFs {
_handle: Option<Handle>,
valid: SetattrValid,
) -> io::Result<(libc::stat64, Duration)> {
let mut inode_table = self.inode_table.lock().unwrap();
let mut inode_table = self.inode_table.write().unwrap();
handle_inode_mut_locked(&mut inode_table, &inode, |InodeState { entry, .. }| match entry {
AuthFsEntry::VerifiedNew { editor, attr } => {
check_unsupported_setattr_request(valid)?;
@ -879,7 +879,7 @@ impl FileSystem for AuthFs {
}
fn unlink(&self, _ctx: Context, parent: Self::Inode, name: &CStr) -> io::Result<()> {
let mut inode_table = self.inode_table.lock().unwrap();
let mut inode_table = self.inode_table.write().unwrap();
handle_inode_mut_locked(
&mut inode_table,
&parent,
@ -906,7 +906,7 @@ impl FileSystem for AuthFs {
}
fn rmdir(&self, _ctx: Context, parent: Self::Inode, name: &CStr) -> io::Result<()> {
let mut inode_table = self.inode_table.lock().unwrap();
let mut inode_table = self.inode_table.write().unwrap();
// Check before actual removal, with readonly borrow.
handle_inode_locked(&inode_table, &parent, |inode_state| match &inode_state.entry {
@ -962,7 +962,7 @@ impl FileSystem for AuthFs {
_size: u32,
offset: u64,
) -> io::Result<Self::DirIter> {
let dir_handle_table = self.dir_handle_table.lock().unwrap();
let dir_handle_table = self.dir_handle_table.read().unwrap();
if let Some(entry) = dir_handle_table.get(&handle) {
Ok(DirEntriesSnapshotIterator {
snapshot: entry.clone(),
@ -980,7 +980,7 @@ impl FileSystem for AuthFs {
_flags: u32,
handle: Self::Handle,
) -> io::Result<()> {
let mut dir_handle_table = self.dir_handle_table.lock().unwrap();
let mut dir_handle_table = self.dir_handle_table.write().unwrap();
if dir_handle_table.remove(&handle).is_none() {
unreachable!("Unknown directory handle {}, inode {}", handle, inode);
}
@ -1008,7 +1008,7 @@ impl FileSystem for AuthFs {
st.f_bfree = st.f_bavail;
st.f_ffree = st.f_favail;
// Number of inodes on the filesystem
st.f_files = self.inode_table.lock().unwrap().len() as u64;
st.f_files = self.inode_table.read().unwrap().len() as u64;
Ok(st)
}