[RFC] fanotify_event_info_fid incompatibility

[RFC] fanotify_event_info_fid incompatibility

[Petr Vorel]

Hi Rich,

musl defines struct fanotify_event_info_fid member fsid as fsid_t. This
conflicts with version from Linux kernel, which defines it as __kernel_fsid_t
(musl's fsid_t has int __val[2], kernel's __kernel_fsid_t has int val[2]).

I see commit 32b82cf5 ("fix the fsid_t structure to match name of __val"),
which looks correct to me.

I also think it's wrong, that other libc (at least glibc, uclibc-ng, bionic)
don't define fanotify_event_info_fid and other structs thus users are forced to
use definition from <linux/fanotify.h>. But can be something done with this
incompatibility?

Kind regards,
Petr

Re: [RFC] fanotify_event_info_fid incompatibility

[Petr Vorel]

Hi,

> musl defines struct fanotify_event_info_fid member fsid as fsid_t. This
> conflicts with version from Linux kernel, which defines it as __kernel_fsid_t
> (musl's fsid_t has int __val[2], kernel's __kernel_fsid_t has int val[2]).

> I see commit 32b82cf5 ("fix the fsid_t structure to match name of __val"),
> which looks correct to me.

> I also think it's wrong, that other libc (at least glibc, uclibc-ng, bionic)
> don't define fanotify_event_info_fid and other structs thus users are forced to
> use definition from <linux/fanotify.h>. But can be something done with this
> incompatibility?

BTW this incompatibility was added quite recently, in f67b3c17 ("sys/fanotify.h:
update for linux v5.1") in v1.1.23.

Kind regards,
Petr

Re: [RFC] fanotify_event_info_fid incompatibility

[Szabolcs Nagy]

* Petr Vorel <petr.vorel@gmail.com> [2019-11-12 21:05:20 +0100]:
> musl defines struct fanotify_event_info_fid member fsid as fsid_t. This
> conflicts with version from Linux kernel, which defines it as __kernel_fsid_t
> (musl's fsid_t has int __val[2], kernel's __kernel_fsid_t has int val[2]).
> 
> I see commit 32b82cf5 ("fix the fsid_t structure to match name of __val"),
> which looks correct to me.
> 
> I also think it's wrong, that other libc (at least glibc, uclibc-ng, bionic)
> don't define fanotify_event_info_fid and other structs thus users are forced to
> use definition from <linux/fanotify.h>. But can be something done with this
> incompatibility?

yeah, glibc sys/fanotify.h just includes linux/fanotify.h
which uses linux types instead of libc ones, this is a
common pattern and there is no clean solution if users
rely on that

in such cases i try to keep updating sys/foo.h following
linux/foo.h changes, but in musl i try to use libc types
(e.g. if a field is __u64 then passing a pointer to it as
uint64_t* may not be valid so the glibc way is quite
problematic)

glibc fsid_t uses __val but the __kernel_fsid_t has val,
musl could use a different type instead of fsid_t for
fanotify that has __val, but it's a bit ugly, is there a
reason to access fsid_t.val?

Re: [RFC] fanotify_event_info_fid incompatibility

[Petr Vorel]

Hi,

> * Petr Vorel <petr.vorel@gmail.com> [2019-11-12 21:05:20 +0100]:
> > musl defines struct fanotify_event_info_fid member fsid as fsid_t. This
> > conflicts with version from Linux kernel, which defines it as __kernel_fsid_t
> > (musl's fsid_t has int __val[2], kernel's __kernel_fsid_t has int val[2]).

> > I see commit 32b82cf5 ("fix the fsid_t structure to match name of __val"),
> > which looks correct to me.

> > I also think it's wrong, that other libc (at least glibc, uclibc-ng, bionic)
> > don't define fanotify_event_info_fid and other structs thus users are forced to
> > use definition from <linux/fanotify.h>. But can be something done with this
> > incompatibility?

> yeah, glibc sys/fanotify.h just includes linux/fanotify.h
> which uses linux types instead of libc ones, this is a
> common pattern and there is no clean solution if users
> rely on that

> in such cases i try to keep updating sys/foo.h following
> linux/foo.h changes, but in musl i try to use libc types
> (e.g. if a field is __u64 then passing a pointer to it as
> uint64_t* may not be valid so the glibc way is quite
> problematic)

Although I understand a reasons for using fsid_t instead of __kernel_fsid_t (and
generally the approach of updating sys/foo.h), I still think that trying to
define a kernel structure in libc but using different members isn't really user
friendly :(.

> glibc fsid_t uses __val but the __kernel_fsid_t has val,
> musl could use a different type instead of fsid_t for
> fanotify that has __val, but it's a bit ugly, is there a
> reason to access fsid_t.val?

LTP fanotify tests access it. Understand, that LTP is not typical user-space
application (+ we can handle it either with autotools detection or just use
<linux/fanotify.h> instead of <sys/fanotify.h>).

Kind regards,
Petr

[1] https://github.com/linux-test-project/ltp/tree/master/testcases/kernel/syscalls/fanotify

Re: [RFC] fanotify_event_info_fid incompatibility

[Szabolcs Nagy]

* Petr Vorel <petr.vorel@gmail.com> [2019-11-12 21:58:31 +0100]:

> Hi,
> 
> > * Petr Vorel <petr.vorel@gmail.com> [2019-11-12 21:05:20 +0100]:
> > > musl defines struct fanotify_event_info_fid member fsid as fsid_t. This
> > > conflicts with version from Linux kernel, which defines it as __kernel_fsid_t
> > > (musl's fsid_t has int __val[2], kernel's __kernel_fsid_t has int val[2]).
> 
> > > I see commit 32b82cf5 ("fix the fsid_t structure to match name of __val"),
> > > which looks correct to me.
> 
> > > I also think it's wrong, that other libc (at least glibc, uclibc-ng, bionic)
> > > don't define fanotify_event_info_fid and other structs thus users are forced to
> > > use definition from <linux/fanotify.h>. But can be something done with this
> > > incompatibility?
> 
> > yeah, glibc sys/fanotify.h just includes linux/fanotify.h
> > which uses linux types instead of libc ones, this is a
> > common pattern and there is no clean solution if users
> > rely on that
> 
> > in such cases i try to keep updating sys/foo.h following
> > linux/foo.h changes, but in musl i try to use libc types
> > (e.g. if a field is __u64 then passing a pointer to it as
> > uint64_t* may not be valid so the glibc way is quite
> > problematic)
> 
> Although I understand a reasons for using fsid_t instead of __kernel_fsid_t (and
> generally the approach of updating sys/foo.h), I still think that trying to
> define a kernel structure in libc but using different members isn't really user
> friendly :(.
> 
> > glibc fsid_t uses __val but the __kernel_fsid_t has val,
> > musl could use a different type instead of fsid_t for
> > fanotify that has __val, but it's a bit ugly, is there a
> > reason to access fsid_t.val?
> 
> LTP fanotify tests access it. Understand, that LTP is not typical user-space
> application (+ we can handle it either with autotools detection or just use
> <linux/fanotify.h> instead of <sys/fanotify.h>).
> 
> Kind regards,
> Petr
> 
> [1] https://github.com/linux-test-project/ltp/tree/master/testcases/kernel/syscalls/fanotify

this test code has internal apis like

static inline void fanotify_get_fid(const char *path, __kernel_fsid_t *fsid,
				    struct file_handle *handle)

which will not work if fsid has any other type than __kernel_fsid_t,
so matching the field name is not enough, you need the linux type here.

mixing linux and libc types have lot of problems, so i'd suggest to
just use linux headers instead of libc ones to test this api.

there are other solutions (like replicating the struct definitions
with different names and memcpy the api struct to that before access),
but it can't be completely clean, because there is a fundamental
conflict. (unfortunately that conflict is documented: the linux man
pages document the linux types as public api even though they don't
mix well with libc types)

Re: [RFC] fanotify_event_info_fid incompatibility

[Petr Vorel]

Hi,

> > [1] https://github.com/linux-test-project/ltp/tree/master/testcases/kernel/syscalls/fanotify

> this test code has internal apis like

> static inline void fanotify_get_fid(const char *path, __kernel_fsid_t *fsid,
> 				    struct file_handle *handle)

> which will not work if fsid has any other type than __kernel_fsid_t,
> so matching the field name is not enough, you need the linux type here.
Yep, that's exactly the problem. But I didn't ask to find a solution for LTP
(that's not difficult), I wanted to raise the problem for musl developers to
reconsider that incompatibility. The only thing I'm not sure, if any other 
application will want to access fsid member (searching on sources.debian.org and
github.com I found only fatrace [2], but it don't touch val/__val, so probably not).

> mixing linux and libc types have lot of problems, so i'd suggest to
> just use linux headers instead of libc ones to test this api.

> there are other solutions (like replicating the struct definitions
> with different names and memcpy the api struct to that before access),
> but it can't be completely clean, because there is a fundamental
> conflict. (unfortunately that conflict is documented: the linux man
> pages document the linux types as public api even though they don't
> mix well with libc types)
Yep, known problem [3].

Kind regards,
Petr

[2] https://sources.debian.org/src/fatrace/0.15-1/fatrace.c/?hl=149#L149
[3] https://sourceware.org/glibc/wiki/Synchronizing_Headers