8000 mount: `user` option is sometimes ignored when used with `X-mount.mkdir` · Issue #3575 · util-linux/util-linux · GitHub
[go: up one dir, main page]

Skip to content

mount: user option is sometimes ignored when used with X-mount.mkdir #3575

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

Open
huntekye opened this issue May 17, 2025 · 7 comments · May be fixed by #3597
Open

mount: user option is sometimes ignored when used with X-mount.mkdir #3575

huntekye opened this issue May 17, 2025 · 7 comments · May be fixed by #3597

Comments

@huntekye
Copy link

With the following line in /etc/fstab:

LABEL=FOO_FS    /home/<me>/FOO_FS    btrfs    defaults,user,X-mount.mkdir=0700   0 0

I sometimes get the following error:

mount: /home/<me>/FOO_FS: must be superuser to use mount.

Specifically, if the target directory doesn't already exist, the first time I call mount, it creates the target directory, and then exits, complaining about permissions as above. If I call mount a second time, without deleting the directory that it made the first time, it mounts the filesystem without any problem. I also checked if changing the order of the two options makes a difference, but X-mount.mkdir=0700,user has the same behavior.

(For completeness):

I think the expected behavior would be that with user option present, mount should not raise errors due to not being the superuser, and that with X-mount.mkdir option present, mount should create the target directory if needed.

In case it's relevant, the full line in my fstab is usually

LABEL=FOO_FS    /home/<me>/FOO_FS    btrfs    defaults,rw,noauto,noatime,user,compress=zstd:3,ssd,user_subvol_rm_allowed,X-mount.owner=<me>,X-mount.mode=0700,X-mount.mkdir=0700    0 0

which has the same behavior as the shorter line. (I tested both the shorter version, and this longer version).

→ mount --version
mount from util-linux 2.41 (libmount 2.41.0: btrfs, verity, namespaces, idmapping, fd-based-mount, statmount, assert, debug)
@echoechoin
Copy link
Contributor

I have tested that the mount() function's arguments remain consistent across two consecutive command executions.

# prepare
echo@debian ~/u/build> sudo chown root:root mount; sudo chmod u+s mount
echo@debian ~/u/build> rm /home/echo/FOO_FS -rf

# firstly
echo@debian ~/u/build> ./mount /home/echo/FOO_FS
mount(2) [source=/dev/nvme0n2, target=/home/echo/FOO_FS, type=btrfs, flags=0x0000000e, options=<none>]
mount: /home/echo/FOO_FS: must be superuser to use mount.
       dmesg(1) may have more information after failed mount system call.

# second
echo@debian ~/u/build> ./mount /home/echo/FOO_FS
mount(2) [source=/dev/nvme0n2, target=/home/echo/FOO_FS, type=btrfs, flags=0x0000000e, options=<none>]

Perhaps it is the behavior of a system call mount.

@huntekye
Copy link
Author

I wanted to try strace to get a better idea of what's going wrong, and I think I've gotten it to behave correctly with the command

# strace --user=<me> --output=<a file> mount LABEL=FOO_FS

(I don't really have any clue why it was necessary to log in as root and fake my user in this way, but running strace mount normally resulted in the mount always failing due to permissions.)

I've attached two traces that I hope might be helpful: mount_fail_trace_from_root.txt is the output when trying to mount as my user when the target directory doesn't already exist, and mounting fails; mount_success_trace_from_root.txt is the successful mount when the target directory already exists.

The contents of the traces are a little hard for me to follow, but it looks like the key difference is at line 126 of mount_fail_trace_from_root.txt and line 104 of mount_success_trace_from_root.txt: In the failed mount, it seems that opening the btrfs executable fails with EPERM, while in the successful mount it works fine.

So the problem seems a little more subtle than I thought at first, which is to say I have no idea what would cause a failure to read the btrfs executable.

Here's some more info on my system:

→ ls -lh /usr/bin/btrfs
-rwxr-xr-x 1 root root 1.3M Mar 27 06:58 /usr/bin/btrfs
→ btrfs --version
btrfs-progs v6.14
-EXPERIMENTAL -INJECT -STATIC +LZO +ZSTD +UDEV +FSVERITY +ZONED CRYPTO=libgcrypt
→ uname -a
Linux <hostname> 6.14.5-arch1-1 #1 SMP PREEMPT_DYNAMIC Sat, 03 May 2025 13:34:12 +0000 x86_64 GNU/Linux

And, as is apparent from the traces, the filesystem I'm trying to mount is a LUKS partition in this case, though I wouldn't expect that to be important in this case.

mount_fail_trace_from_root.txt
mount_success_trace_from_root.txt

@echoechoin
Copy link
Contributor
echoechoin commented May 20, 2025

Call sanitize_paths -> canonicalize_path_restricted -> realpath will cause canonicalize_path_restricted return NULL if there is no mount target directory. And then, mount will call suid_dropsetting euid back to uid instead of 0. This process let mount command has no permission to call system-call mount.

if (mnt_context_is_restricted(cxt) && sanitize_paths(cxt) != 0)
	suid_drop(cxt);

@echoechoin
Copy link
Contributor

should I move sanitize_paths after __mkdir hook ? @karelzak . Or other solution?

@karelzak
Copy link
Collaborator

The sanitize_paths prevents non-root users from accessing "foreign" paths. I have doubts that supporting user and X-mount.mkdir together is a good idea.

The problem is that sanitize_paths() is called at the beginning (for security reasons), 8000 and before mount/libmount, it reads fstab, so it has no clue about the mount options. IF we move the check after the mkdir hook, then there will be a huge area for possible security issues.

It would be better to document in man page that user-mount need the target directory and mkdir is not allowed.

@huntekye
Copy link
Author

I feel like it's not entirely clear to me how much of a security hole allowing user and X-mount.mkdir together should have to make, since, if I understand correctly, the presence of user implies that the mount options are being read from the fstab, and so the target directory should also be the one from the fstab. I think that should mean that even making the directory with elevated privileges should be safe, assuming the fstab was written well. But I could definitely see how that could be taking things too far. On the other hand, given the presence of user in the fstab, shouldn't it be fine to allow making a new directory with the user's normal privileges?

Or maybe the real problem would be allowing user in the fstab together with X-mount.mkdir on the command line; that does seem sort of risky. If X-mount.mkdir is in the fstab though, I wouldn't expect it to cause any problems.

To me it seems like moving the logic of X-mount.mkdir to before sanitize_paths (or possibly as something checked in a branch from within that function) would make sense. As for the code structure, the contents of hook_mkdir.c is short enough that perhaps it could be moved/duplicated to part of mount.c?

@karelzak
Copy link
Collaborator

The original motivation for sanitize_paths() is to avoid situations where a non-root user uses mount(8) to verify the existence of any file or directory in the system, trigger anything (automount, inotify, etc.), or exploit any libmount bug. For example, in a very old version without sanitize_paths(), mount(8) returned different error messages if the path existed and was not configured in fstab. Thus, using "mount /root/foo" to detect if a file was present was possible.

I think it would be safer to keep sanitize_paths() at the beginning of all mount processes, but perhaps we can make the function smarter. For example, we can accept incomplete paths if all existing elements of the path are accessible to the current user and the last directory is writable. The hook_mkdir.c should be extended to consider non-root users.

@echoechoin echoechoin linked a pull request May 28, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
0