Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20191112212947.GJ25646@port70.net>
Date: Tue, 12 Nov 2019 22:29:47 +0100
From: Szabolcs Nagy <nsz@...t70.net>
To: Petr Vorel <petr.vorel@...il.com>
Cc: musl@...ts.openwall.com, Rich Felker <dalias@...ifal.cx>
Subject: Re: [RFC] fanotify_event_info_fid incompatibility

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

> Hi,
> 
> > * Petr Vorel <petr.vorel@...il.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)

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.