[RFC][PATCH] uapi: Remove the inclusion of linux/mount.h from uapi/linux/fs.h

[RFC][PATCH] uapi: Remove the inclusion of linux/mount.h from uapi/linux/fs.h

[David Howells]

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

Re: [RFC][PATCH] uapi: Remove the inclusion of linux/mount.h from uapi/linux/fs.h

[Arnd Bergmann]

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

Re: [RFC][PATCH] uapi: Remove the inclusion of linux/mount.h from uapi/linux/fs.h

[Christian Brauner]

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>

Re: [RFC][PATCH] uapi: Remove the inclusion of linux/mount.h from uapi/linux/fs.h

[Florian Weimer]

* 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

Re: [RFC][PATCH] uapi: Remove the inclusion of linux/mount.h from uapi/linux/fs.h

[Ian Kent]

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