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>
Came up on the container branch and not sure why it triggers there
only, but malloc is not needed and snprintf should check for the
max size.
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
Came up on the container branch and not sure why it triggers there
only, but malloc is not needed and snprintf should check for the
max size.
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
As annotated by CodeQL, argc already test for above and cannot be
smaller than 3, no need to check again.
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
* master:
Change use_subtype_prefix type to bool in fuse_mnt_build_source()
lib mount: fix fsfd leakage on error
lib mount: include linux/mount.h in mount_i_linux.h
lib mount: no need to fsconfig mtab options
Fix ENODEV fallback logic for block devices with fsname
Update the condition when to use synchronous init
fuse_daemonize_early: Rename is_active to is_used, add new is_active
Add a missing NULL check for x_mtab_opts in mount_fuse()
fusermount: Fix heap buffer overflow in perform_mount()
lib/mount.c: Restore "subtype#fsname" format for legacy kernel fallback
fuse: No need to call strlen
Just a minor API cleanup from yesterday, change type from int to
bool and add comment why the argument is needed.
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
Clean up fsfd and related resources before falling back to old
mount API if ms_flags_to_mount_attrs() fails. Otherwise the created
detached fuse connection will hang there and won't be freed.
Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
mtab options is a superset of kernel options (passed to kernel via
mount(2) or fsconfig(2)), with additional mtab-only options like
"user=" or "-n".
After switching to the new mount API, kernel options are already
configured by fsconfig. Re-running fsconfig on mtab options causes
options to be configured twice.
Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
The ENODEV fallback logic in both lib/mount.c and util/fusermount.c
incorrectly combined two conditions, causing block devices with fsname
to be treated as if they had no fsname.
Original behavior (before commit 50fa7f0bc2):
if (mo->fsname) {
if (!mo->blkdev)
sprintf(source, "%s#%s", mo->subtype, mo->fsname);
// else: keep existing source (fsname for blkdev)
} else {
strcpy(source, type);
}
This handled three cases:
1. fsname + non-blkdev -> "subtype#fsname"
2. fsname + blkdev -> fsname (unchanged)
3. no fsname -> type
Previous patches incorrectly changed this to:
if (mo->fsname && !mo->blkdev) {
source = fuse_mnt_build_source()
} else {
source = strdup(type);
}
This only handled two cases, causing case 2 (fsname + blkdev) to be
treated as case 3 (no fsname).
This patch restores the three-way logic.
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
The previous condition in session_start_sync_init() if to
use synchronous init was not right yet - we actually want
sync-init by default when fuse_daemonize_earl_start is
used. But users also need to able to override (at least
we need to be able to override for our own tests).
So add an enum with the values of auto (default),
enabled and disabled.
Noticed due to a PR from Jingbo Xu <jefflexu@linux.alibaba.com>
that added the workaround to enable, which not supposed to be
needed. The logic should work on its own when fuse_daemonize_early_start
is used.
This also exports fuse_session_set_sync_init (and renames it from
fuse_session_want_sync_init()).
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
In passthrough_hp.cc in main() the err_out4 condition would
have printed an invalid warning and program exit even
under regular umount, because the underlying check acted
on daemonize.active and daemonize.daemonized, although it
was supposed to act on daemonize.active only.
Fix that by renaming the current _is_active() to _is_used()
and by introducing a new fuse_daemonize_is_active(), which
only checks for 'daemonize.daemonized'.
Suggested-by: Jingbo Xu <jefflexu@linux.alibaba.com>
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
Commit 50fa7f0bc2 ("Move fuse_mnt_build_{source,type} to mount_util.c"
introduced a heap buffer overflow in the ENODEV fallback
path in fusermount.c. The prepare_mount() function now allocates source
with only strlen(fsname) + 1 bytes via fuse_mnt_build_source(), but
perform_mount() writes strlen(subtype) + 1 + strlen(fsname) + 1 bytes
when building the legacy "subtype#fsname" format.
Fix by removing source/type from struct mount_params and building them
as local variables in perform_mount() with correct allocation using the
new use_subtype_prefix parameter added in the previous commit.
The source and type strings are now:
- Built in perform_mount() with proper size allocation
- Returned via output parameters to the caller
- No longer stored in the mount_params structure
Fixes: 50fa7f0bc2 ("Move fuse_mnt_build_{source,type} to mount_util.c")
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
Commit 50fa7f0bc2 ("Move fuse_mnt_build_{source,type} to mount_util.c"
changed the ENODEV fallback behavior from using
"subtype#fsname" format to just "fsname" for the mount source.
This adds a use_subtype_prefix parameter to fuse_mnt_build_source() to
control whether the legacy "subtype#fsname" format should be used. This
centralizes the source string building logic and avoids code duplication
in the ENODEV fallback paths.
Original behavior (before 50fa7f0bc2):
if (mo->fsname) {
if (!mo->blkdev)
sprintf(source, "%s#%s", mo->subtype, mo->fsname);
} else {
strcpy(source, type);
}
Behavior after 50fa7f0bc2 (unintended change):
source = fuse_mnt_build_source(mo->fsname, NULL, devname);
// Returns just fsname, not "subtype#fsname"
Behavior before 50fa7f0bc2 is restored again.
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
This only checks for "." and "..". This can be done cheaper than
calling strlen. According to godbolt not much code is required for
this.
Signed-off-by: Volker Lendecke <vl@samba.org>
Remove dangerous strcpy calls with memcpy based on the lengths we used
to compute the dynamic structure size.
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
In theory, fuservicemount can be called with argv[0] == NULL, so let's
protect ourselves against that.
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Darrick J. Wong