mount -o remount fails with EINVAL on kernel version 5.12.0

[boryas]

As far as I can tell, any kind of invocation of mount -o remount fails (regardless of ro/rw/other options, or ext4/btrfs)

The error output on the command line is:

lt-mount: /root/util-linux/test.mnt: mount point not mounted or bad option.
       dmesg(1) may have more information after failed mount system call.

Nothing interesting in dmesg, though.

To reproduce:
I built a vanilla 5.12.0 kernel off the v5.12 tag in Linus's tree, and checked out and built util-linux from source.
Sample reproducer script using loop devices and ext4 here: https://pastebin.com/mt0RcYrF
You can invoke it like this:
MOUNT_CMD=<path-to-freshly-built-mount> ./repro.sh
with MOUNT_CMD unset, it will use mount and find it in $PATH.

Some more details:
I also tested it on a branch derived from 5.19 and that succeeded.
When using btrfs, bpftrace shows that we call btrfs_remount and that returns 0.
strace shows that the syscall returning EINVAL is mount_setattr
hacking up the mount command to always use the fallback to the old syscall also "fixes" the issue. (hack patch: https://pastebin.com/VBCd7srR)

[t-8ch]

Given that 5.12.0 has not received bugfixes for quite some time, this could already be fixed in still maintained kernels.

[boryas]

That seems likely, given that it worked on 5.19 (admittedly, I haven't tested on vanilla 5.19, but happy to try that too)

What is the mount command's backwards compatibility goal here? It is currently not backwards compatible, which is fine if that is the intent, but based on the changelog, I got the impression that the intent was to be backwards compatible:

supports new file descriptors based mount kernel API. This change is very aggressive to libmount code, but hopefully, it does not introduce regressions in traditional mount(8) behavior. The library is still backward compatible with old kernels and can fall back to classic mount(2) API if necessary.

Is that only referring to the new syscall existing at all vs. being buggy? What if all mounting was broken on 5.12 not just remount? FWIW, on systems that mount a read only rootfs in an initrd then remount it rw before switch_root, no remount is as bad as no mount.

[t-8ch]

The problem here is that the bug could be either in 5.12 or util-linux.
Supporting compatibility with software that itself is not maintained anymore is hard and probably not worth the effort.
Especially as hopefully there are only very few users trying to run bleeding edge userspace on unsupported kernels.

TLDR: if it's possible to properly handle it can be done, but somebody probably has to bring up the time to do so.

[boryas]

I honestly don't see why it matters where the bug is. Either the mount command is backwards compatible with 5.12.0 or it isn't. It's advertised as seeking to be, but there is a totally valid and documented behavior (-o remount) which used to work and no longer works with the new version. That's not backward compatible, regardless of which end of the API is not upholding its specification.

I have a good enough reproducer that I can probably bisect/pull patches related to this stuff until it starts working like 5.19 (or 5.15.114 which is an LTS kernel that I also tested and did not reproduce). But if that's the recommendation for me, I think util-linux should be honest about backwards compatibility of the mount command w.r.t. kernel version.

Alternatively, would you accept a patch along the lines of an attempted fallback to mount() if mount_setattr() returns EINVAL during remount?

[t-8ch]

It does not say that it is compatible with the 5.12 mount API.
It is compatible with the "new filedescriptors based mount kernel API".

We don't know if the issue is in util-linux or if the new API was broken in 5.12 and has been fixed in the meantime.
Nobody wants util-linux to be incompatible but the effort to fix it in a kernel that nobody should be using anyways has to come from somewhere.

would you accept a patch along the lines of an attempted fallback to mount() if mount_setattr() returns EINVAL during remount?

(I'm not the maintainer, so I can't accept patches or give you definitive answers)

Those patches would most likely be accepted.
However this would not work well with the architecture of libmount.
By the time mount_setattr() has been attempted it's to late to fall back mount() with the current architecture AFAIK.
So it would be a bigger change.

[t-8ch]

Maybe it's useful to have an environment variable or mount flag to force the old mount logic so users can work around any issues until proper fixes have been implemented.

[karelzak]

There is also possible to add a condition when the new API will be ignored; we already use it for btrfs+selinux. In the worst case, we can use if (get_linux_version() == KERNEL_VERSION(5, 12, 0)) return 1; in hook_prepare() for the new API.

[karelzak]

I'll play with it on Monday.

[boryas]

Thanks for both of your help. Please let me know if I can help with implementing or testing anything.

Apologies in advance for a bit of pedantry here, but I do think it is important to be clear about issues like backwards compatibility.

It does not say that it is compatible with the 5.12 mount API.

The changelog, and I do not think I am mangling the context or meaning:

The library is still backward compatible with old kernels and can fall back to classic mount(2) API if necessary.

Again, thank you for your help, and have a lovely weekend.

[boryas]

Update: I believe I have found the issue. The mount_setattr syscall was failing on the check of the validity of the attr_set and attr_clr variables coming in from userspace. ATTR_NOSYMFOLLOW was present in the attributes passed in, but not in the valid list on 5.12.0

https://lore.kernel.org/linux-fsdevel/20210601135515.126639-1-brauner@kernel.org/

Adds it to the kernel's list of valid attrs, and cherry-picking that kernel change fixes the issue. I'm not sure if that information is useful for a workaround. Perhaps it would be possible to not emit that attr on kernels before that patch?

[karelzak]

@boryas, please, can you try #2296 for your kernel?

[boryas]

@karelzak Confirmed on a fresh test host running 5.12 that it succeeds with #2296 and fails on the 'HEAD^' of that branch. Thanks very much for the workaround!

[karelzak]

I have updated the patch, which now requires LIBMOUNT_FORCE_MOUNT2=always (rather than "yes").