linux-kernel - Re: [PATCH v3 3/3] selftests: pidfd: add tests for PIDFD_SELF

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <97fbdb19-fe46-45e2-883c-2ec0e0e6416c@lucifer.local>
Date: Wed, 16 Oct 2024 23:30:26 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Shuah Khan <skhan@...uxfoundation.org>
Cc: Christian Brauner <christian@...uner.io>, Shuah Khan <shuah@...nel.org>,
        "Liam R . Howlett" <Liam.Howlett@...cle.com>,
        Suren Baghdasaryan <surenb@...gle.com>,
        Vlastimil Babka <vbabka@...e.cz>, pedro.falcato@...il.com,
        linux-kselftest@...r.kernel.org, linux-mm@...ck.org,
        linux-fsdevel@...r.kernel.org, linux-api@...r.kernel.org,
        linux-kernel@...r.kernel.org, Oliver Sang <oliver.sang@...el.com>
Subject: Re: [PATCH v3 3/3] selftests: pidfd: add tests for PIDFD_SELF_*

On Wed, Oct 16, 2024 at 11:06:34PM +0100, Lorenzo Stoakes wrote:
[sniip]
> >
> > The import will be fine and you can control that with -I flag in
> > the makefile. Remove these and try to get including linux/pidfd.h
> > working.
>
> I just tried this and it's not fine :) it immediately broke the build as
> pidfd.h imports linux/fcntl.h which conflicts horribly with system headers
> on my machine.
>
> For instance f_owner_ex gets redefined among others and fails the build e..g:
>
> /usr/include/asm-generic/fcntl.h:155:8: error: redefinition of ‘struct f_owner_ex’
>   155 | struct f_owner_ex {
>       |        ^~~~~~~~~~
> In file included from /usr/include/bits/fcntl.h:61,
>                  from /usr/include/fcntl.h:35,
>                  from pidfd_test.c:6:
> /usr/include/bits/fcntl-linux.h:274:8: note: originally defined here
>   274 | struct f_owner_ex
>       |        ^~~~~~~~~~
>
> It seems only one other test tries to do this as far as I can tell (I only
> did a quick grep), so it's not at all standard it seems.
>
> This issue occurred even when I used make headers_install to create
> sanitised user headers and added them to the include path.
>
> A quick google suggests linux/fcntl.h (imported by this pidfd.h uapi
> header) and system fcntl.h is a known thing. Slightly bizarre...
>
> I tried removing the <fcntl.h> include and that resulted in <sys/mount.h>
> conflicting:
>
> In file included from /usr/include/fcntl.h:35,
>                  from /usr/include/sys/mount.h:24,
>                  from pidfd.h:17,
>                  from pidfd_test.c:22:
> /usr/include/bits/fcntl.h:35:8: error: redefinition of ‘struct flock’
>    35 | struct flock
>       |        ^~~~~
> In file included from /tmp/hdr/include/asm/fcntl.h:1,
>                  from /tmp/hdr/include/linux/fcntl.h:5,
>                  from /tmp/hdr/include/linux/pidfd.h:7,
>                  from pidfd.h:6:
> /usr/include/asm-generic/fcntl.h:195:8: note: originally defined here
>   195 | struct flock {
>       |        ^~~~~
>
> So I don't think I can actually work around this, at least on my system,
> and I can't really sensibly submit a patch that I can't run on my own
> machine :)
>
> I may be missing something here.
>

[snip]

Some added data:

OK so I asked people on fedi to compile the following locally (also a
variant with _GNU_SOURCE being defined):

	#include <linux/pidfd.h>
	#include <fcntl.h>

	int main(void) {}

And they are all encountering the same issue as I am on a number of
different distros (ordering of includes doesn't seem to matter either).

So this seems like a known-broken thing.

And we can't really isolate inclusion of this file since all the tests
interact directly with defines from it.

So it seems the only solution is the workaround I suggested previously I
think with the header guard define hack.

[snip]