Hi,

We're seeing issues in autofs and xfstests whereby linux/mount.h (the UAPI
version) as included indirectly by linux/fs.h is conflicting with
sys/mount.h (there's a struct and an enum).

Would it be possible to just remove the #include from linux/fs.h (as patch
below) and rely on those hopefully few things that need mount flags that don't
use the glibc header for them working around it by configuration?

David
---
uapi: Remove the inclusion of linux/mount.h from uapi/linux/fs.h
    
Remove the inclusion of <linux/mount.h> from uapi/linux/fs.h as it
interferes with definitions in sys/mount.h - but linux/fs.h is needed by
various things where mount flags and structs are not.

Note that this will likely have the side effect of causing some build
failures.

Reported-by: Ian Kent <raven@themaw.net>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Alexander Viro <viro@zeniv.linux.org.uk>
cc: Christian Brauner <christian@brauner.io>
cc: linux-fsdevel@vger.kernel.org
cc: linux-api@vger.kernel.org
---
 include/uapi/linux/fs.h |    5 -----
 1 file changed, 5 deletions(-)

diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index bdf7b404b3e7..7a2597ac59ed 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -17,11 +17,6 @@
 #include <linux/fscrypt.h>
 #endif
 
-/* Use of MS_* flags within the kernel is restricted to core mount(2) code. */
-#if !defined(__KERNEL__)
-#include <linux/mount.h>
-#endif
-
 /*
  * It's silly to have NR_OPEN bigger than NR_FILE, but you can change
  * the file limit at runtime and only root can increase the per-process

* David Howells:

> We're seeing issues in autofs and xfstests whereby linux/mount.h (the UAPI
> version) as included indirectly by linux/fs.h is conflicting with
> sys/mount.h (there's a struct and an enum).
>
> Would it be possible to just remove the #include from linux/fs.h (as patch
> below) and rely on those hopefully few things that need mount flags that don't
> use the glibc header for them working around it by configuration?

Wasn't <linux/mount.h> split from <linux/fs.h> relatively recently, and
userspace is probably using <linux/fs.h> to get the mount flag
definitions?

In retrospect, it would have been better to add the new fsmount stuff to
a separate header file, so that we could include that easily from
<sys/mount.h> on the glibc side.  Adhemerval posted a glibc patch to
fake that (for recent compilers):

  [PATCH] linux: Fix sys/mount.h usage with kernel headers
  <https://sourceware.org/pipermail/libc-alpha/2022-August/141316.html>

I think it should work reliably, so that's probably the direction we are
going to move in.

We'll backport this to 2.36, and distributions will pick it up.

Thanks,
Florian

On 10/8/22 17:26, Florian Weimer wrote:
> * David Howells:
>
>> We're seeing issues in autofs and xfstests whereby linux/mount.h (the UAPI
>> version) as included indirectly by linux/fs.h is conflicting with
>> sys/mount.h (there's a struct and an enum).
>>
>> Would it be possible to just remove the #include from linux/fs.h (as patch
>> below) and rely on those hopefully few things that need mount flags that don't
>> use the glibc header for them working around it by configuration?
> Wasn't <linux/mount.h> split from <linux/fs.h> relatively recently, and
> userspace is probably using <linux/fs.h> to get the mount flag
> definitions?

Not sure myself but this is in the user space kernel includes

and sys/mount.h has pretty much what linux/mount.h has plus a

few function declarations. It's almost a complete duplication.


The reality is that the enum declaration could be changed to

#defines (not a preferred approach) which leaves only the

struct mount_attr which is the difficult one to resolve.


>
> In retrospect, it would have been better to add the new fsmount stuff to
> a separate header file, so that we could include that easily from
> <sys/mount.h> on the glibc side.  Adhemerval posted a glibc patch to
> fake that (for recent compilers):
>
>    [PATCH] linux: Fix sys/mount.h usage with kernel headers
>    <https://sourceware.org/pipermail/libc-alpha/2022-August/141316.html>
>
> I think it should work reliably, so that's probably the direction we are
> going to move in.

Looked a lot more complicated than I thought it could be, enough

that I can't say if it will work so I'll take your word for it.


Ian

On Mon, Aug 08, 2022 at 02:17:35PM +0100, David Howells wrote:
> Hi,
> 
> We're seeing issues in autofs and xfstests whereby linux/mount.h (the UAPI
> version) as included indirectly by linux/fs.h is conflicting with
> sys/mount.h (there's a struct and an enum).

(The linux/mount.h and sys/mount.h is painful for userspace too btw.)

> 
> Would it be possible to just remove the #include from linux/fs.h (as patch
> below) and rely on those hopefully few things that need mount flags that don't
> use the glibc header for them working around it by configuration?
> 
> David
> ---
> uapi: Remove the inclusion of linux/mount.h from uapi/linux/fs.h
>     
> Remove the inclusion of <linux/mount.h> from uapi/linux/fs.h as it
> interferes with definitions in sys/mount.h - but linux/fs.h is needed by
> various things where mount flags and structs are not.
> 
> Note that this will likely have the side effect of causing some build
> failures.
> 
> Reported-by: Ian Kent <raven@themaw.net>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Alexander Viro <viro@zeniv.linux.org.uk>
> cc: Christian Brauner <christian@brauner.io>
> cc: linux-fsdevel@vger.kernel.org
> cc: linux-api@vger.kernel.org
> ---

Yeah, I think this is ok.
Reviewed-by: Christian Brauner (Microsoft) <brauner@kernel.org>

On Mon, Aug 8, 2022 at 3:17 PM David Howells <dhowells@redhat.com> wrote:
>
> Hi,
>
> We're seeing issues in autofs and xfstests whereby linux/mount.h (the UAPI
> version) as included indirectly by linux/fs.h is conflicting with
> sys/mount.h (there's a struct and an enum).
>
> Would it be possible to just remove the #include from linux/fs.h (as patch
> below) and rely on those hopefully few things that need mount flags that don't
> use the glibc header for them working around it by configuration?
>
> David
> ---
> uapi: Remove the inclusion of linux/mount.h from uapi/linux/fs.h
>
> Remove the inclusion of <linux/mount.h> from uapi/linux/fs.h as it
> interferes with definitions in sys/mount.h - but linux/fs.h is needed by
> various things where mount flags and structs are not.
>
> Note that this will likely have the side effect of causing some build
> failures.
>
> Reported-by: Ian Kent <raven@themaw.net>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Alexander Viro <viro@zeniv.linux.org.uk>
> cc: Christian Brauner <christian@brauner.io>
> cc: linux-fsdevel@vger.kernel.org
> cc: linux-api@vger.kernel.org

I think that's probably ok. Looking through the results from

https://codesearch.debian.net/search?q=MS_RDONLY
combined with the list of results from
https://codesearch.debian.net/search?q=linux/fs.h

I found 95 packages that mention both /somewhere/ in the code, see
below for a complete list. I did not try an exhaustive search but found
that almost all of these reference the two in different files.

The only counterexample I found is

https://sources.debian.org/src/trinity/1.9+git20200331.4d2343bd18c7b-2/syscalls/mount.c/?hl=13#L13

so this will likely break and have to get fixed to includle <linux/mount.h> or
<sys/mount.h> instead of linux/fs.h, but that is probably acceptable.
There may be others like it, but not a ton of them.

      Arnd

8<---
android-framework-23
android-platform-art
android-platform-frameworks-base
android-platform-system-core
android-platform-system-extras
android-platform-tools
apparmor
audit
avfs
bazel-bootstrap
bcachefs-tools
bpfcc
busybox
cargo
cde
ceph
chromium
criu
cryptmount
docker.io
e2fsprogs
elogind
emscripten
falcosecurity-libs
firefox
firefox-esr
fstransform
gcc-10
gcc-11
gcc-12
gcc-9
gcc-snapshot
gfarm
glibc
glusterfs
gnumach
golang-1.11
golang-1.15
golang-1.16
golang-1.17
golang-1.18
golang-1.19
golang-github-containers-storage
golang-golang-x-sys
golang-inet-netstack
hashrat
hurd
icingadb
kfreebsd-10
klibc
kubernetes
kvmtool
libblockdev
libexplain
librsvg
libvirt
linux
linux-apfs-rw
lxc
manpages-l10n
mergerfs
mozjs91
mtd-utils
network-manager
nilfs-tools
ntfs-3g
ocfs2-tools
ostree
partclone
ploop
qemu
quota
rust-libc
rustc
sash
snapd
strace
stress-ng
suricata
systemd
systemtap
testdisk
thunderbird
tiny-initramfs
tomoyo-tools
toybox
trinity
u-boot
uclibc
umview
util-linux
virtualbox
webhook
zfs-linux
zulucrypt