Skip to content

Commit cf4c814

Browse files
committed
passthrough: open without passthrough when the kernel refuses a backing file
do_open() and create() failed the OPEN or CREATE when registering the backing file failed. Without CAP_SYS_ADMIN, or with a host file on overlayfs and the default max_stack_depth of 1, every open of a regular file on the mount failed, even though init() had enabled passthrough. Open the handle without passthrough instead, so the daemon serves its IO. That is safe because registering only fails when the inode has no backing id yet, so none of its handles is open in passthrough mode. It logs the first refusal as a warning and later ones at debug level. EPERM would fail every registration, so it turns passthrough off for the instance. An inode that still has handles with a backing id keeps sharing it. The kernel fails an open of an inode in passthrough mode unless the open uses the same backing file. An inode can now have handles with and without a backing id, so each handle records whether it holds one, and only those drop a reference on release. Before, releasing any handle of the inode dropped one, and the id could close while a passthrough handle was still open. Signed-off-by: Ilan Joselevich <personal@ilanjoselevich.com>
1 parent 26da2d4 commit cf4c814

2 files changed

Lines changed: 174 additions & 40 deletions

File tree

‎crates/fuse-backend-passthrough/src/lib.rs‎

Lines changed: 162 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -282,12 +282,19 @@ struct KernelPassthrough {
282282

283283
#[cfg(target_os = "linux")]
284284
impl KernelPassthrough {
285+
/// Get the backing id of `inode` for a new handle if the inode has one.
286+
fn share(&self, inode: Inode) -> Option<u32> {
287+
// Do not expect poisoned lock here, so safe to unwrap().
288+
let mut ids = self.ids.lock().unwrap();
289+
let (id, handles) = ids.get_mut(&inode)?;
290+
*handles += 1;
291+
Some(*id)
292+
}
293+
285294
/// Get the backing id of `inode` for a new handle, registering `file` if the inode has none.
286295
fn acquire(&self, inode: Inode, file: &File) -> io::Result<u32> {
287-
// Do not expect poisoned lock here, so safe to unwrap().
288-
if let Some((id, handles)) = self.ids.lock().unwrap().get_mut(&inode) {
289-
*handles += 1;
290-
return Ok(*id);
296+
if let Some(id) = self.share(inode) {
297+
return Ok(id);
291298
}
292299
// Register the file without the lock, so that a slow ioctl doesn't hold up the opens and
293300
// releases of other inodes.
@@ -357,14 +364,17 @@ struct HandleData {
357364
inode: Inode,
358365
file: File,
359366
open_flags: RwLock<u32>,
367+
/// Whether the handle holds a reference to its inode's backing id.
368+
has_backing_id: bool,
360369
}
361370

362371
impl HandleData {
363-
fn new(inode: Inode, file: File, flags: u32) -> Self {
372+
fn new(inode: Inode, file: File, flags: u32, has_backing_id: bool) -> Self {
364373
HandleData {
365374
inode,
366375
file,
367376
open_flags: RwLock::new(flags),
377+
has_backing_id,
368378
}
369379
}
370380

@@ -421,16 +431,15 @@ impl HandleMap {
421431
self.handles.write().unwrap().insert(handle, Arc::new(data));
422432
}
423433

424-
fn release(&self, handle: Handle, inode: Inode) -> io::Result<()> {
434+
fn release(&self, handle: Handle, inode: Inode) -> io::Result<Arc<HandleData>> {
425435
// Do not expect poisoned lock here, so safe to unwrap().
426436
let mut handles = self.handles.write().unwrap();
427437

428438
if let btree_map::Entry::Occupied(e) = handles.entry(handle) {
429439
if e.get().inode == inode {
430440
// We don't need to close the file here because that will happen automatically when
431441
// the last `Arc` is dropped.
432-
e.remove();
433-
return Ok(());
442+
return Ok(e.remove());
434443
}
435444
}
436445

@@ -518,6 +527,9 @@ pub struct PassthroughFs<S: BitmapSlice + Send + Sync = ()> {
518527
// was called and `init` was called with `FsOptions::PASSTHROUGH`.
519528
#[cfg(target_os = "linux")]
520529
passthrough: AtomicBool,
530+
// Whether a backing file the kernel refused was logged at warn level already.
531+
#[cfg(target_os = "linux")]
532+
backing_refusal_logged: AtomicBool,
521533

522534
// Whether to offload the synchronous handlers to the blocking thread
523535
// pool with the async-io feature, see
@@ -594,6 +606,8 @@ impl<S: BitmapSlice + Send + Sync> PassthroughFs<S> {
594606
kernel_passthrough: None,
595607
#[cfg(target_os = "linux")]
596608
passthrough: AtomicBool::new(false),
609+
#[cfg(target_os = "linux")]
610+
backing_refusal_logged: AtomicBool::new(false),
597611
#[cfg(feature = "async-io")]
598612
async_thread_pool_enabled: AtomicBool::new(false),
599613
#[cfg(feature = "async-io")]
@@ -677,16 +691,56 @@ impl<S: BitmapSlice + Send + Sync> PassthroughFs<S> {
677691

678692
/// Register the backing file of a new handle of `inode` if kernel passthrough is enabled, and
679693
/// return its backing id.
694+
///
695+
/// If the kernel refuses the backing file, the handle opens without passthrough, so the
696+
/// daemon serves its IO. That is safe because registering only fails when the inode has no
697+
/// backing id yet, so none of its handles is open in passthrough mode. `EPERM` means the
698+
/// daemon lacks `CAP_SYS_ADMIN` and every registration would fail, so it turns passthrough
699+
/// off.
700+
///
701+
/// With passthrough off, an inode that still has handles with a backing id keeps sharing it.
702+
/// The kernel fails an open of an inode in passthrough mode unless the open uses the same
703+
/// backing file.
680704
#[cfg(target_os = "linux")]
681-
fn acquire_backing_id(&self, inode: Inode, file: &File) -> io::Result<Option<u32>> {
682-
match &self.kernel_passthrough {
683-
Some(kp) if self.passthrough.load(Ordering::Relaxed) => {
684-
kp.acquire(inode, file).map(Some)
705+
fn acquire_backing_id(&self, inode: Inode, file: &File) -> Option<u32> {
706+
let kp = self.kernel_passthrough.as_ref()?;
707+
if !self.passthrough.load(Ordering::Relaxed) {
708+
return kp.share(inode);
709+
}
710+
match kp.acquire(inode, file) {
711+
Ok(id) => Some(id),
712+
Err(e) if e.raw_os_error() == Some(libc::EPERM) => {
713+
if self.passthrough.swap(false, Ordering::Relaxed) {
714+
warn!(
715+
"fuse: kernel passthrough needs CAP_SYS_ADMIN, serving IO in the daemon: {}",
716+
e
717+
);
718+
}
719+
None
720+
}
721+
Err(e) => {
722+
// Log the first refusal as a warning, and the rest at debug level.
723+
let level = if self.backing_refusal_logged.swap(true, Ordering::Relaxed) {
724+
log::Level::Debug
725+
} else {
726+
log::Level::Warn
727+
};
728+
log::log!(
729+
level,
730+
"fuse: can't register the backing file of inode {}, opening it without passthrough: {}",
731+
inode,
732+
e
733+
);
734+
None
685735
}
686-
_ => Ok(None),
687736
}
688737
}
689738

739+
#[cfg(not(target_os = "linux"))]
740+
fn acquire_backing_id(&self, _inode: Inode, _file: &File) -> Option<u32> {
741+
None
742+
}
743+
690744
/// Get the list of file descriptors which should be reserved across live upgrade.
691745
pub fn keep_fds(&self) -> Vec<RawFd> {
692746
vec![self.proc_self_fd.as_raw_fd()]
@@ -983,12 +1037,14 @@ impl<S: BitmapSlice + Send + Sync> PassthroughFs<S> {
9831037
// `HandleMap::clear()`. If `release()` fails the cached cookie (if
9841038
// any) is left behind; that is harmless and it will be dropped by
9851039
// `destroy()`.
986-
self.handle_map.release(handle, inode)?;
1040+
let data = self.handle_map.release(handle, inode)?;
9871041
self.handle_map.remove_cookie(handle);
9881042
// Directories and handles opened without passthrough have no backing id to release.
989-
#[cfg(target_os = "linux")]
990-
if let Some(kp) = &self.kernel_passthrough {
991-
kp.release(inode);
1043+
if data.has_backing_id {
1044+
#[cfg(target_os = "linux")]
1045+
if let Some(kp) = &self.kernel_passthrough {
1046+
kp.release(inode);
1047+
}
9921048
}
9931049
Ok(())
9941050
}
@@ -1941,6 +1997,95 @@ mod tests {
19411997
assert_eq!(*files.closed.lock().unwrap(), vec![1, 2]);
19421998
}
19431999

2000+
#[test]
2001+
#[cfg(target_os = "linux")]
2002+
fn test_kernel_passthrough_falls_back() {
2003+
/// Answers registrations with `results` in order, and records the closed ids.
2004+
struct Files {
2005+
results: Mutex<Vec<io::Result<u32>>>,
2006+
calls: AtomicU64,
2007+
closed: Mutex<Vec<u32>>,
2008+
}
2009+
impl Files {
2010+
fn new(mut results: Vec<io::Result<u32>>) -> Arc<Self> {
2011+
results.reverse();
2012+
Arc::new(Files {
2013+
results: Mutex::new(results),
2014+
calls: AtomicU64::new(0),
2015+
closed: Mutex::new(Vec::new()),
2016+
})
2017+
}
2018+
}
2019+
impl BackingFiles for Files {
2020+
fn open(&self, _fd: BorrowedFd<'_>) -> io::Result<u32> {
2021+
self.calls.fetch_add(1, Ordering::Relaxed);
2022+
self.results.lock().unwrap().pop().unwrap()
2023+
}
2024+
fn close(&self, backing_id: u32) -> io::Result<()> {
2025+
self.closed.lock().unwrap().push(backing_id);
2026+
Ok(())
2027+
}
2028+
}
2029+
fn passthrough_fs(tmpdir: &TempDir, files: Arc<Files>) -> PassthroughFs<()> {
2030+
let mut fs = PassthroughFs::<()>::new(Config {
2031+
do_import: false,
2032+
root_dir: tmpdir.as_path().to_string_lossy().to_string(),
2033+
..Default::default()
2034+
})
2035+
.unwrap();
2036+
fs.set_backing_files(files);
2037+
fs.import().unwrap();
2038+
assert!(fs
2039+
.init(FsOptions::PASSTHROUGH)
2040+
.unwrap()
2041+
.contains(FsOptions::PASSTHROUGH));
2042+
fs
2043+
}
2044+
let ctx = Context::default();
2045+
let createin = CreateIn {
2046+
flags: libc::O_CREAT as u32 | libc::O_RDWR as u32,
2047+
mode: 0o644,
2048+
umask: 0,
2049+
fuse_flags: 0,
2050+
};
2051+
let name = CString::new("file").unwrap();
2052+
2053+
// A refused backing file opens without passthrough, and the next open of the inode tries
2054+
// again.
2055+
let tmpdir = TempDir::new().unwrap();
2056+
let files = Files::new(vec![Err(io::Error::from_raw_os_error(libc::ELOOP)), Ok(7)]);
2057+
let fs = passthrough_fs(&tmpdir, files.clone());
2058+
let (entry, h1, opts1, id1) = fs.create(&ctx, ROOT_ID, &name, createin).unwrap();
2059+
assert!(!opts1.contains(OpenOptions::PASSTHROUGH));
2060+
assert_eq!(id1, None);
2061+
let (h2, opts2, id2) = fs
2062+
.open(&ctx, entry.inode, libc::O_RDONLY as u32, 0)
2063+
.unwrap();
2064+
assert!(opts2.contains(OpenOptions::PASSTHROUGH));
2065+
assert_eq!(id2, Some(7));
2066+
// Releasing the handle without an id leaves the id of the other one alone.
2067+
fs.release(&ctx, entry.inode, 0, h1.unwrap(), false, false, None)
2068+
.unwrap();
2069+
assert!(files.closed.lock().unwrap().is_empty());
2070+
fs.release(&ctx, entry.inode, 0, h2.unwrap(), false, false, None)
2071+
.unwrap();
2072+
assert_eq!(*files.closed.lock().unwrap(), vec![7]);
2073+
2074+
// EPERM turns passthrough off, so the next open doesn't try again.
2075+
let tmpdir = TempDir::new().unwrap();
2076+
let files = Files::new(vec![Err(io::Error::from_raw_os_error(libc::EPERM))]);
2077+
let fs = passthrough_fs(&tmpdir, files.clone());
2078+
let (entry, _, opts1, id1) = fs.create(&ctx, ROOT_ID, &name, createin).unwrap();
2079+
assert!(!opts1.contains(OpenOptions::PASSTHROUGH));
2080+
assert_eq!(id1, None);
2081+
let (_, opts2, id2) = fs
2082+
.open(&ctx, entry.inode, libc::O_RDONLY as u32, 0)
2083+
.unwrap();
2084+
assert!(!opts2.contains(OpenOptions::PASSTHROUGH));
2085+
assert_eq!(id2, None);
2086+
assert_eq!(files.calls.load(Ordering::Relaxed), 1);
2087+
}
2088+
19442089
#[test]
19452090
#[cfg(target_os = "linux")]
19462091
fn test_kernel_passthrough_racing_opens() {

‎crates/fuse-backend-passthrough/src/sync_io.rs‎

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -601,18 +601,17 @@ impl<S: BitmapSlice + Send + Sync> PassthroughFs<S> {
601601
let file = self.open_inode(inode, flags as i32)?;
602602
drop(killpriv);
603603

604-
let data = HandleData::new(inode, file, flags);
605604
// Only regular files pass through, `opendir` also gets here.
606-
#[cfg(target_os = "linux")]
607605
let backing_id = if flags & (libc::O_DIRECTORY as u32) == 0 {
608-
self.acquire_backing_id(inode, &data.file)?
606+
self.acquire_backing_id(inode, &file)
609607
} else {
610608
None
611609
};
612-
#[cfg(not(target_os = "linux"))]
613-
let backing_id = None;
614610
let handle = self.next_handle.fetch_add(1, Ordering::Relaxed);
615-
self.handle_map.insert(handle, data);
611+
self.handle_map.insert(
612+
handle,
613+
HandleData::new(inode, file, flags, backing_id.is_some()),
614+
);
616615

617616
let opts = if flags & (libc::O_DIRECTORY as u32) == 0 {
618617
self.file_open_options(flags, backing_id)
@@ -679,7 +678,7 @@ impl<S: BitmapSlice + Send + Sync> PassthroughFs<S> {
679678
self.handle_map.get(handle, inode)
680679
} else {
681680
let file = self.open_inode(inode, flags | libc::O_DIRECTORY)?;
682-
Ok(Arc::new(HandleData::new(inode, file, flags as u32)))
681+
Ok(Arc::new(HandleData::new(inode, file, flags as u32, false)))
683682
}
684683
}
685684

@@ -694,7 +693,7 @@ impl<S: BitmapSlice + Send + Sync> PassthroughFs<S> {
694693
self.handle_map.get(handle, inode)
695694
} else {
696695
let file = self.open_inode(inode, flags)?;
697-
Ok(Arc::new(HandleData::new(inode, file, flags as u32)))
696+
Ok(Arc::new(HandleData::new(inode, file, flags as u32, false)))
698697
}
699698
}
700699
}
@@ -1058,24 +1057,14 @@ impl<S: BitmapSlice + Send + Sync> FileSystem for PassthroughFs<S> {
10581057
}
10591058
};
10601059

1061-
#[allow(unused_mut)]
10621060
let mut backing_id = None;
10631061
let ret_handle = if !self.no_open.load(Ordering::Relaxed) {
1064-
let data = HandleData::new(entry.inode, file, args.flags);
1065-
#[cfg(target_os = "linux")]
1066-
{
1067-
backing_id = match self.acquire_backing_id(entry.inode, &data.file) {
1068-
Ok(id) => id,
1069-
Err(e) => {
1070-
// Drop the lookup count of the entry that the kernel won't get.
1071-
let mut inodes = self.inode_map.get_map_mut();
1072-
self.forget_one(&mut inodes, entry.inode, 1);
1073-
return Err(e);
1074-
}
1075-
};
1076-
}
1062+
backing_id = self.acquire_backing_id(entry.inode, &file);
10771063
let handle = self.next_handle.fetch_add(1, Ordering::Relaxed);
1078-
self.handle_map.insert(handle, data);
1064+
self.handle_map.insert(
1065+
handle,
1066+
HandleData::new(entry.inode, file, args.flags, backing_id.is_some()),
1067+
);
10791068
Some(handle)
10801069
} else {
10811070
None

0 commit comments

Comments
 (0)