Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow wrapping an existing /dev/fuse file descriptor #304

Merged
merged 1 commit into from
Oct 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/channel.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
use std::{fs::File, io, os::unix::prelude::AsRawFd, sync::Arc};
use std::{
fs::File,
io,
os::{
fd::{AsFd, BorrowedFd},
unix::prelude::AsRawFd,
},
sync::Arc,
};

use libc::{c_int, c_void, size_t};

Expand All @@ -8,6 +16,12 @@ use crate::reply::ReplySender;
#[derive(Debug)]
pub struct Channel(Arc<File>);

impl AsFd for Channel {
fn as_fd(&self) -> BorrowedFd<'_> {
self.0.as_fd()
}
}

impl Channel {
/// Create a new communication channel to the kernel driver by mounting the
/// given path. The kernel driver will delegate filesystem operations of
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub use reply::{
ReplyStatfs, ReplyWrite,
};
pub use request::Request;
pub use session::{BackgroundSession, Session, SessionUnmounter};
pub use session::{BackgroundSession, Session, SessionACL, SessionUnmounter};
#[cfg(feature = "abi-7-28")]
use std::cmp::max;
#[cfg(feature = "abi-7-13")]
Expand Down
64 changes: 39 additions & 25 deletions src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use libc::{EAGAIN, EINTR, ENODEV, ENOENT};
use log::{info, warn};
use nix::unistd::geteuid;
use std::fmt;
use std::os::fd::{AsFd, BorrowedFd, OwnedFd};
use std::path::{Path, PathBuf};
use std::sync::{Arc, Mutex};
use std::thread::{self, JoinHandle};
Expand All @@ -31,10 +32,15 @@ pub const MAX_WRITE_SIZE: usize = 16 * 1024 * 1024;
/// up to MAX_WRITE_SIZE bytes in a write request, we use that value plus some extra space.
const BUFFER_SIZE: usize = MAX_WRITE_SIZE + 4096;

#[derive(Debug, Eq, PartialEq)]
pub(crate) enum SessionACL {
#[derive(Default, Debug, Eq, PartialEq)]
/// How requests should be filtered based on the calling UID.
pub enum SessionACL {
/// Allow requests from any user. Corresponds to the `allow_other` mount option.
All,
/// Allow requests from root. Corresponds to the `allow_root` mount option.
RootAndOwner,
/// Allow requests from the owning UID. This is FUSE's default mode of operation.
#[default]
Owner,
}

Expand All @@ -46,9 +52,7 @@ pub struct Session<FS: Filesystem> {
/// Communication channel to the kernel driver
pub(crate) ch: Channel,
/// Handle to the mount. Dropping this unmounts.
mount: Arc<Mutex<Option<Mount>>>,
/// Mount point
mountpoint: PathBuf,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make this optional instead of removing it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the utility of getting the mountpoint back out, after you just passed it in? Especially if it returns an Option<PathBuf>?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The getter doesn't seem useful, but it seems nice to have the logging

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the logging back.

mount: Arc<Mutex<Option<(PathBuf, Mount)>>>,
/// Whether to restrict access to owner, root + owner, or unrestricted
/// Used to implement allow_root and auto_unmount
pub(crate) allowed: SessionACL,
Expand All @@ -64,6 +68,12 @@ pub struct Session<FS: Filesystem> {
pub(crate) destroyed: bool,
}

impl<FS: Filesystem> AsFd for Session<FS> {
fn as_fd(&self) -> BorrowedFd<'_> {
self.ch.as_fd()
}
}

Comment on lines +71 to +76
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to have a mount_to_fd() function similar to mount() that instead returns the FD. That better separates the concerns of creating the mountpoint, which won't even need to have a Filesystem implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just for symmetry - I'm not using it, so I can remove it if you don't like it. Note that the stdlib equivalents all support this (File, various sockets, etc).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would someone use it though?

The reason it doesn't seem right to me is that a Session represents a mounted filesystem, borrowing its fd, and reading/writing it would interfere with the Filesystem that is mounted and feels like it breaks the encapsulation.

Copy link
Contributor Author

@colinmarc colinmarc Oct 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would someone use it though?

By passing it into mount(2)/fsconfig(2) to mount the session somewhere.

In my use case, you need to mount first and then create the session. But it's not hard to imagine someone needing to do it the other way around.

EDIT: I just thought of another one. You could unset CLOEXEC or do some other fcntl stuff on the session if you needed to.

EDIT2: You could also use it to call fuse ioctls, perhaps ones that aren't supported by this library yet (eg file passthrough).

Copy link
Contributor Author

@colinmarc colinmarc Oct 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zooming out a bit:

I would like to submit that FUSE is a very low-level concept, and it's often used as a hack-on-demand. So people using it probably know what they're doing, and may have complicated requirements. A fuse session is a /dev/fuse file descriptor - therefore it fulfills the requirements of AsFd.

I do see the concerns around breaking the encapsulation, but Channel is not actually stateful, right? So it's really very similar to UnixStream or whatever. If you call AsFd on a UnixStream and drop down to nix::io::write or whatever, you probably have a very specific use case that the stdlib authors didn't think of.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good points. Thanks for explaining the use case!

impl<FS: Filesystem> Session<FS> {
/// Create a new session by mounting the given filesystem to the given mountpoint
pub fn new<P: AsRef<Path>>(
Expand Down Expand Up @@ -100,8 +110,7 @@ impl<FS: Filesystem> Session<FS> {
Ok(Session {
filesystem,
ch,
mount: Arc::new(Mutex::new(Some(mount))),
mountpoint: mountpoint.to_owned(),
mount: Arc::new(Mutex::new(Some((mountpoint.to_owned(), mount)))),
allowed,
session_owner: geteuid().as_raw(),
proto_major: 0,
Expand All @@ -111,9 +120,21 @@ impl<FS: Filesystem> Session<FS> {
})
}

/// Return path of the mounted filesystem
pub fn mountpoint(&self) -> &Path {
&self.mountpoint
/// Wrap an existing /dev/fuse file descriptor. This doesn't mount the
/// filesystem anywhere; that must be done separately.
pub fn from_fd(filesystem: FS, fd: OwnedFd, acl: SessionACL) -> Self {
let ch = Channel::new(Arc::new(fd.into()));
Session {
filesystem,
ch,
mount: Arc::new(Mutex::new(None)),
allowed: acl,
session_owner: geteuid().as_raw(),
proto_major: 0,
proto_minor: 0,
initialized: false,
destroyed: false,
}
}

/// Run the session loop that receives kernel requests and dispatches them to method
Expand Down Expand Up @@ -177,7 +198,7 @@ impl<FS: Filesystem> Session<FS> {
#[derive(Debug)]
/// A thread-safe object that can be used to unmount a Filesystem
pub struct SessionUnmounter {
mount: Arc<Mutex<Option<Mount>>>,
mount: Arc<Mutex<Option<(PathBuf, Mount)>>>,
}

impl SessionUnmounter {
Expand Down Expand Up @@ -210,40 +231,38 @@ impl<FS: Filesystem> Drop for Session<FS> {
self.filesystem.destroy();
self.destroyed = true;
}
info!("Unmounted {}", self.mountpoint().display());

if let Some((mountpoint, _mount)) = std::mem::take(&mut *self.mount.lock().unwrap()) {
info!("unmounting session at {}", mountpoint.display());
}
}
}

/// The background session data structure
pub struct BackgroundSession {
/// Path of the mounted filesystem
pub mountpoint: PathBuf,
/// Thread guard of the background session
pub guard: JoinHandle<io::Result<()>>,
/// Object for creating Notifiers for client use
#[cfg(feature = "abi-7-11")]
sender: ChannelSender,
/// Ensures the filesystem is unmounted when the session ends
_mount: Mount,
_mount: Option<Mount>,
}

impl BackgroundSession {
/// Create a new background session for the given session by running its
/// session loop in a background thread. If the returned handle is dropped,
/// the filesystem is unmounted and the given session ends.
pub fn new<FS: Filesystem + Send + 'static>(se: Session<FS>) -> io::Result<BackgroundSession> {
let mountpoint = se.mountpoint().to_path_buf();
#[cfg(feature = "abi-7-11")]
let sender = se.ch.sender();
// Take the fuse_session, so that we can unmount it
let mount = std::mem::take(&mut *se.mount.lock().unwrap());
let mount = mount.ok_or_else(|| io::Error::from_raw_os_error(libc::ENODEV))?;
let mount = std::mem::take(&mut *se.mount.lock().unwrap()).map(|(_, mount)| mount);
let guard = thread::spawn(move || {
let mut se = se;
se.run()
});
Ok(BackgroundSession {
mountpoint,
guard,
#[cfg(feature = "abi-7-11")]
sender,
Expand All @@ -253,7 +272,6 @@ impl BackgroundSession {
/// Unmount the filesystem and join the background thread.
pub fn join(self) {
let Self {
mountpoint: _,
guard,
#[cfg(feature = "abi-7-11")]
sender: _,
Expand All @@ -274,10 +292,6 @@ impl BackgroundSession {
// thread_scoped::JoinGuard
impl fmt::Debug for BackgroundSession {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
write!(
f,
"BackgroundSession {{ mountpoint: {:?}, guard: JoinGuard<()> }}",
self.mountpoint
)
write!(f, "BackgroundSession {{ guard: JoinGuard<()> }}",)
}
}
Loading