memfs_write passed offset and size straight to Inode::write_content,
which computes offset + size to size the content vector. A negative
offset converts to a huge unsigned value in that expression and aborts
the resize; an end position past the vector's index ceiling truncates
on the resize/iterator arithmetic (size_t/ptrdiff_t), which on a 32-bit
build is below the off_t range the kernel can hand us.
memfs_read only guarded offset >= content_size(). On a 64-bit build the
unsigned compare incidentally rejects a negative offset, but on 32-bit
the comparison is signed, so a negative offset slips through, truncates
in (size_t)offset, and indexes before the content buffer.
Validate at both handlers before touching the content store: reject a
negative offset with EINVAL, and in write reject an end position past
PTRDIFF_MAX (the std::vector size/index limit on both 32- and 64-bit
builds) with EFBIG.
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
Add a -o fuse_dio mount option that, on open and create, sets fi->direct_io
and fi->parallel_direct_writes. parallel_direct_writes only takes effect
alongside direct_io, so both are enabled together under the one option.
Useful for benchmarking the direct-I/O path, where the kernel no longer
serializes writes per inode and no longer clamps reads at i_size.
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
For benchmarking the FUSE interface itself rather than memfs's std::vector
content management, add a -o null_io mount option. Writes still consume
the incoming buffer (folding it into a volatile sink so the reads are not
elided) but discard it; reads return random data served from a scratch
buffer filled once at startup. The content store is never touched on
either path.
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
When memfs_rename overwrites an existing target, it called
remove_child(newname) -- which deletes the matched Dentry -- and then
dereferenced existing_dentry again via existing_dentry->get_inode() to
decrement the replaced inode's nlink, reading the freed Dentry: a
use-after-free. Resolve the target
inode before remove_child and use the saved pointer afterwards; the
inode stays alive through the Inodes map reference.
The same branch also decremented newparent's nlink twice when the target
was a directory: once explicitly and again inside remove_child(), which
already decrements a directory's parent. A directory-over-directory
rename therefore left newparent's link count one too low (rmdir
decrements the parent exactly once, via remove_child). Drop the
redundant explicit decrement.
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
memfs_link replies with fuse_reply_entry, which hands the kernel a
new lookup reference on the inode, but it only bumped nlink and never
bumped nlookup. Every other entry-replying handler (lookup, create,
mkdir) accounts for that reference. Without it, the kernel's accumulated
forget count for a hard-linked inode exceeds nlookup, and dec_lookup()
underflows and panics with "Lookup count mismatch detected" -- readily
reproduced by a concurrent stress loop that hard-links files. Increment
nlookup on the successful link reply.
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
memfs_rename locked the source then destination directory positionally.
Two concurrent renames in opposite directions could therefore deadlock
if the global inodes_mutex were ever relaxed to allow them to run
in parallel. Today the exclusive Inodes.lock() held for the whole
operation serializes them, so the deadlock is masked, but the positional
order is a latent hazard.
Order the two directory locks by inode number -- always lock the lower
ino first -- so any two renames acquire the per-directory locks in the
same order. The ordering locals are declared with the other rename
locals, before the first goto, because the validation-failure goto
would otherwise cross an initialized declaration into its scope. The
operation body still acts on the directories by role; both are locked
regardless of which was taken first.
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
Inodes::find() returned a raw Inode* after releasing its shared lock,
so every handler dereferenced an inode with nothing preventing a
concurrent forget from erasing and deleting it -- a use-after-free.
Related defects: forget erased an inode at nlookup==0 regardless of
nlink, freeing still-linked inodes; readdir dereferenced raw Dentry
pointers snapshotted at opendir; lookup dereferenced a Dentry after
dropping the parent lock; forget_multi resolved and decremented without
a lock and never erased at zero.
Make the inode table own std::shared_ptr<Inode> and have
find()/find_locked()/create() return shared_ptr, so a handler holds a
strong reference for the whole operation. A directory link (Dentry) now
holds a shared_ptr too, so a linked inode cannot be freed while
reachable. get_children()/DirHandle snapshot strong references, fixing
the readdir UAF. lookup copies the child's shared_ptr under the parent
lock before using it. An inode is erased only once it has no kernel
references and no links (nlookup==0 && nlink==0); forget and the
rewritten forget_multi both apply that guard under inodes_mutex.
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
memfs_link took inodes_mutex exclusively via Inodes.lock(), then resolved
the source and parent inodes with Inodes.find(), which takes a shared lock
on the same mutex -- a self-deadlock on a non-recursive shared_mutex. It
then called parent_inode->add_child(), which re-locks the parent inode
mutex already held via parent_inode->lock() -- a second self-deadlock.
The new Dentry was owned by a unique_ptr whose raw pointer was handed
to the parent and then freed at scope exit, leaving the parent with
a dangling dentry.
Resolve both inodes with find_locked() (inodes_mutex already held)
and link with add_child_locked() (parent inode mutex already held).
Heap-allocate the Dentry and hand ownership to the parent, which frees
it in remove_child(), matching create()/mkdir(). On add_child_locked()
failure, delete the dentry and undo the inc_nlink() before replying
the error.
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
memfs_mkdir's out_cleanup path called erase_locked() without holding
inodes_mutex, an unsynchronized map erase on the failure path. Use the
locking erase() variant.
memfs_create ignored add_child()'s return value. Its early find_child()
EEXIST check and the later add_child() are two separate parent-lock
acquisitions, so two concurrent create("foo") calls can both pass
the check, both allocate distinct inodes, and both reach add_child();
the loser gets EEXIST, which was silently discarded. That leaked the
loser's Dentry and orphaned its inode (left in the map, no link) while
still replying success. Capture the return and, on failure, delete
the dentry and erase the orphan inode before replying the error. The
inode was never linked or returned to the kernel, so the unconditional
erase() is correct.
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
Inodes::size() locked a separate std::mutex that protected nothing while
reading the inode map, which is guarded by inodes_mutex. Concurrent
create/erase could therefore reshape the map under an unsynchronized
size() read. Take a shared lock on inodes_mutex instead and drop the
stray mutex member, which had no other use.
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
The per-inode state is split across two mutexes for concurrency:
Inode::mutex (content, mtime, dentries, nlookup) and Inode::attr_mutex
(mode, uid, gid, atime). A chmod/chown/atime update should not serialize
behind a write() resizing the content vector, so the two domains keep
separate locks.
The original split was inconsistent, which was the actual bug:
write_content() updated content+mtime under mutex while truncate()
did the same under attr_mutex, and get_attr() read content.size()
under attr_mutex. So content and mtime were each touched under both
locks depending on the path, and a getattr could race a concurrent
write resizing the content. get_mode()/get_mtime() read mutable fields
with no lock at all; get_children() and is_empty() iterated the dentry
vector unlocked.
Give each field a single owning lock. content, mtime, size: mutex (truncate
and set_mtime move onto it). mode, uid, gid, atime: attr_mutex. ctime
is set once at construction and never mutated, so it stays lock-free.
get_attr() needs a consistent cross-domain snapshot, so it is the one
place that holds both, always in the order mutex -> attr_mutex; no
setter holds both, so there is no ABBA risk. memfs_setattr drops
its outer lock()/unlock() because the setters and the final get_attr
now self-lock; keeping it would deadlock the non-recursive mutex. A
concurrent getattr may then observe a partially-applied setattr, which
is acceptable for an example filesystem.
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
As discussed on https://github.com/libfuse/libfuse/issues/1515, libfuse has
a very light dependency on libnuma, only using very light wrappers over
mmap, mbind, and munmap. Remove the libnuma dependency by calling those
syscalls directly.
Populate memory after mmap to avoid SIGBUS later.
Signed-off-by: Benjamin Peterson <benjamin@python.org>
Bernd received a complaint via github that the "unmount" command in the
new fuse service API is prone to malicious symlink creation races
because the umount2 call doesn't use UMOUNT_NOFOLLOW. Therefore, if you
can trick a fuse service into asking the mount service helper to undo
the mount just after you've replaced the mount with a symlink to a
sensitive mount (e.g. /sys) then you whack a system hard.
I don't know why this unmounting functionality exists in libfuse, and
apparently Bernd doesn't like it. Since the fuse service API has not
yet been released, let's just rip out the command and pretend it never
existed.
Fixes: 5d8b9a39a1 ("mount_service: add systemd socket service mounting helper")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Amazingly, permission for the use of the "blkdev" mount option (aka
mounting with fuseblk.SUBTYPE instead of fuse.SUBTYPE) rests solely on a
getuid()==0 check in userspace. That /might/ be an indication that the
kernel also won't allow this, but as that's the criteria, let's
advertise fuseblk explicitly so that a fuse server can discover the
capability before calling fuse_service_session_mount().
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
If the fuse server tries to open a file with
FUSE_SERVICE_REQUEST_FILE_QUIET, that means they don't want the mount
helper to emit any error messages about the file open() failing. We did
this for the main open() call, but not for any of the code in prep_bdev.
Oops.
Fixes: 5d8b9a39a1 ("mount_service: add systemd socket service mounting helper")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Codex complains that the protocol checking here is too stringent. For
example, if the mount helper advertises min=1,max=2 but libfuse inside
the container is older and only supports protocol version 1, the second
check results in service mount failure when we could just use v1.
Fix this by requiring that maxver >= minver, and then looking for a lack
of overlap between what libfuse supports and what the helper advertised.
Fixes: 5d8b9a39a1 ("mount_service: add systemd socket service mounting helper")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Codex noticed a discrepancy between the new fsmount code that Bernd
wrote and my port of the even newer mount service code to use the
helpers that Bernd wrote. Specifically, set_fsconfig_ms_flags only
clears bits from MS_FLAGS if there's no corresponding MOUNT_ATTR_ flag,
whereas ms_flags_to_mount_attrs always clears them.
In other words, set_fsconfig_ms_flags MUST be called before
ms_flags_to_mount_attrs or we can lose the 'ro' option. Fix this.
Fixes: 7211953256 ("mount_service: use the fsmount API helpers from mount_fsmount.c")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
If mount_service_handle_mntopts_cmd previously looked at the mount
options to screen for allow_{other,root}, then tokstr (aka oc->value)
has been modified and not put back. This causes truncation of the mount
options passed to fsconfig, so let's use the snapshot that we took
previously. Codex noticed this.
Fixes: 7211953256 ("mount_service: use the fsmount API helpers from mount_fsmount.c")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
The mount(2) flags (i.e. MS_*) are unsigned long. Don't truncate them
in this function by assigning to int.
Fixes: 14cb7b93bb ("Add support for the new linux mount API")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
The pinned-fd TOCTOU fix rejected any mountpoint not owned by the
mounting user, but check_nonroot_dir_access() only enforces ownership
for sticky directories. Restrict the pinned-inode check to match.
Fixes: bad8b22c91 ("fusermount: fix sync-init TOCTOU by mounting on a pinned mountpoint fd")
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
With drop_privileges, mount.fuse3 execs the file system binary only
after dropping all capabilities (CAP_DAC_OVERRIDE included) while
keeping uid 0. The kernel then applies plain DAC checks to the exec, so
a binary under a directory the capability-less process cannot traverse
(e.g. a user-owned mode-0700 home) fails with EACCES and /bin/sh prints
a bare 'Permission denied', surfacing in the tests as 'file system
process terminated prematurely'.
Resolving and opening the binary before the drop was rejected: it would
perform a privileged path resolution and open() on invoker-influenced
input (confused-deputy / privilege-escalation risk). The drop-first,
resolve-after order is preserved.
Instead, pre-flight the upcoming PATH lookup after the drop and fail
with a diagnostic that names the binary and points at mount.fuse3(8).
The check runs with the final credentials, so it is deny-only. The test
detects the same situation up front (uid/gid-aware traversal check) and
skips with a precise reason; root-owned CI checkouts keep running.
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
I disagree with this style annotation - just takes another line
without gaining much.
WARNING:LINE_SPACING: Missing a blank line after declarations
+ int ret = access_executable(test_path);
+ if (ret == 0)
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
The header buffer is described by "struct fuse_uring_req_header" rather
than "struct fuse_ring_ent".
Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
The --sync-init code path validates the mountpoint in
mount_fuse_prepare() (check_perm() chdir()s into the directory and
rewrites the path to ".", or opens a regular-file mountpoint and
rewrites it to /proc/self/fd/N), but that pinned reference is a local
variable that is discarded. mount_fuse_finish_fsmount() later mounts
using the original path string, which move_mount() re-resolves through
whatever the symlinks now say.
An unprivileged user controls the sync-init socket, so
wait_for_signal() gives an unbounded check-to-use window: validate an
attacker-owned directory, swap a parent component to a symlink into
/etc, then signal -- the mount lands on a root-owned directory,
defeating check_nonroot_dir_access(). With user_allow_other this yields
local root via a fake sudoers drop-in.
Pin the validated inode as a single fd (mount_context.mnt_fd) obtained
from the already-chdir'd CWD or the open mountpoint fd, mount onto it
via move_mount() with MOVE_MOUNT_T_EMPTY_PATH (no path re-resolution),
and assert at the mount site that fstat(mnt_fd) still matches the stat
captured during validation. The legacy mount() path and the library
direct-mount path are unchanged (mnt_fd == -1 keeps path resolution).
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
fuse_session_loop_mt_312() unconditionally destroys se->mt_lock before
returning. The caller then invokes fuse_session_destroy(), which
destroys the same mutex again. This is undefined behavior per POSIX
on every MT shutdown.
fuse_session_reset() does not re-initialize mt_lock between these
calls, so the second destroy always operates on an already-destroyed
mutex.
Remove the destroy from fuse_session_loop_mt_312 and let
fuse_session_destroy remain the sole owner of mutex teardown,
consistent with how se->mt_finish (the semaphore) is already handled.
Signed-off-by: Abhinav Agarwal <abhinavagarwal1996@gmail.com>
If fuse_session_mount() succeeds but fuse_session_unmount() is never
called (e.g., on an error path), the mountpoint string allocated by
strdup in fuse_session_mount() is leaked. Free it in
fuse_session_destroy() using the same atomic_exchange pattern as
fuse_session_unmount() to handle concurrent access safely.
Signed-off-by: Abhinav Agarwal <abhinavagarwal1996@gmail.com>
grow_pipe_to_max() opens /proc/sys/fs/pipe-max-size without O_CLOEXEC.
In a multi-threaded FUSE daemon, a concurrent fork+exec (e.g., to
invoke fusermount) between the open() and close() calls would leak this
fd into the child process.
Add the same O_CLOEXEC fallback guard used in mount.c and
fuse_loop_mt.c for portability to older headers.
Signed-off-by: Abhinav Agarwal <abhinavagarwal1996@gmail.com>
The fstat() call on the communication fd from _FUSE_COMMFD was not
checking its return value. On failure (e.g., invalid fd number),
statbuf remains uninitialized and the subsequent S_ISSOCK check
operates on garbage stack data, potentially bypassing the socket
type validation.
Check the return value and fail closed on error.
Signed-off-by: Abhinav Agarwal <abhinavagarwal1996@gmail.com>
fuse_req_getgroups() uses sprintf() to write into a fixed 128-byte
buffer. While the current format string cannot overflow in practice,
using snprintf() with sizeof(path) provides bounds safety as a
defense-in-depth measure.
Signed-off-by: Abhinav Agarwal <abhinavagarwal1996@gmail.com>
fuse_uring_stop() frees the ring pool via fuse_session_destruct_uring()
but does not null se->uring.pool afterwards, leaving a dangling pointer.
While no current code path dereferences the pool after stop, this is a
latent maintenance hazard — any future access would hit freed memory.
The error path in fuse_uring_start() already nulls se->uring.pool on
failure; apply the same discipline in the normal teardown path.
Signed-off-by: Abhinav Agarwal <abhinavagarwal1996@gmail.com>
mount_max is parsed with %i (signed) from /etc/fuse.conf. The value
-1 is the documented sentinel for "no limit", but any other negative
value (e.g., -2 from a typo) passes the mount_max != -1 guard and
makes the mount_count >= mount_max comparison always true, blocking
all non-root FUSE mounts.
Reject values below -1 with a warning rather than silently accepting
them.
Signed-off-by: Abhinav Agarwal <abhinavagarwal1996@gmail.com>
get_mnt_opts() accesses (*mnt_optsp)[l-1] without checking that
l > 0. If the option string is empty (reachable via read-only mount
with no additional options), this is an out-of-bounds read at
index -1.
Signed-off-by: Abhinav Agarwal <abhinavagarwal1996@gmail.com>
After opening the fromfs iconv descriptor, the error check on line 725
tests ic->tofs (already verified above) instead of ic->fromfs. A failed
iconv_open for the fromfs direction is silently ignored, leaving an
invalid descriptor that causes undefined behavior in iconv_convpath.
Signed-off-by: Abhinav Agarwal <abhinavagarwal1996@gmail.com>
Replace bare except clauses with 'except Exception' to avoid
catching system-exiting exceptions like KeyboardInterrupt and
SystemExit, which should not be suppressed.
Fixes: GitHub CodeQL Alerts #15, #16, #17
Rule: py/catch-base-exception
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
Use 'with' statements to ensure files are properly closed even
when exceptions are raised during pytest.raises() contexts.
Fixes: GitHub CodeQL Alerts #18, #19
Rule: py/file-not-closed
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
Move subprocess.check_output() and proc.wait() calls out of assert
statements to avoid side effects being optimized away when running
with Python optimization enabled.
Fixes: GitHub CodeQL Alerts #22-27
Rule: py/side-effect-in-assert
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
Documentation for poll() callback says that:
> The callee is responsible for destroying ph with
> fuse_pollhandle_destroy() when no longer in use.
In fuse_lib_poll() ((struct fuse_lowlevel_ops*)->poll) we need to be more
careful:
1. If get_path_nullok() fails, we need to free fuse_pollhandle
2. If we passed execution down to fuse_fs_poll(), then
it must release fuse_pollhandle resources when no
(struct fuse_operations*)->poll provided by the filesystem driver.
Found this by myself while reading the code as a part of [1] review.
This is not critical at all, because once we return ENOSYS once,
kernel never sends FUSE_POLL again. So memleak is unnoticable in practice.
Alternatively, we can leak if get_path_nullok() fails all the time, but
then we are in a much more serious troubles...
Link: https://github.com/lxc/lxcfs/pull/726 [1]
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
Add support for parsing FUSE_SECCTX extensions sent by the kernel
during file creation operations. This allows userspace filesystems
to receive and persist SELinux/AppArmor security labels on newly
created files.
Add example in passthrough_hp: When creating a file, iterates the fuse
req extension security contexts to find the SELinux security label and
apply it.
Signed-off-by: Kevin Chen <kchen@ddn.com>
test: Make short_tmpdir respect pytest --basetemp
When running libfuse tests in a QEMU-based vmtest environment,
test_passthrough fails with a PermissionError because the guest
restricts operations under /tmp:
FAILED test/test_examples.py::test_passthrough[False-passthrough-False]
PermissionError: [Errno 1] Operation not permitted:
'/tmp/tmp4veumkm0/mnt/tmp/tmp4veumkm0/src/testfile_16'
The short_tmpdir fixture used the custom raii_tmpdir wrapper which
calls tempfile.mkdtemp() and always places directories under /tmp,
ignoring pytest's --basetemp option. Since --basetemp is used to
relocate temporary directories to a writable location in QEMU guests,
the fixture prevents the workaround from taking effect.
Fix it by making raii_tmpdir accept an optional dir argument and
having short_tmpdir check whether --basetemp was passed. When
--basetemp is set, create the temporary directory under its resolved
path obtained via tmp_path_factory.getbasetemp(). Without --basetemp,
fall back to the original behavior to keep paths short enough for
AF_UNIX sun_path limits (104 bytes on FreeBSD/macOS, 108 on Linux).
Signed-off-by: KaFai Wan <kafai.wan@linux.dev>
Bernd Schubert