linux-kernel - Re: [PATCH] uapi/fcntl: conditionally define AT

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: <CAOQ4uxiShq5gPCsRh5ZDNXbG4AGH5XpfHx0HXDWTS+5Y95hieQ@mail.gmail.com>
Date: Mon, 25 Aug 2025 07:58:02 +0200
From: Amir Goldstein <amir73il@...il.com>
To: Randy Dunlap <rdunlap@...radead.org>, Aleksa Sarai <cyphar@...har.com>
Cc: Matthew Wilcox <willy@...radead.org>, linux-kernel@...r.kernel.org, 
	Jeff Layton <jlayton@...nel.org>, Chuck Lever <chuck.lever@...cle.com>, 
	Alexander Aring <alex.aring@...il.com>, Josef Bacik <josef@...icpanda.com>, Jan Kara <jack@...e.cz>, 
	Christian Brauner <brauner@...nel.org>, linux-fsdevel@...r.kernel.org, 
	linux-api@...r.kernel.org
Subject: Re: [PATCH] uapi/fcntl: conditionally define AT_RENAME* macros

On Mon, Aug 25, 2025 at 1:54 AM Randy Dunlap <rdunlap@...radead.org> wrote:
>
>
>
> On 8/24/25 4:21 PM, Matthew Wilcox wrote:
> > On Sun, Aug 24, 2025 at 03:10:55PM -0700, Randy Dunlap wrote:
> >> Don't define the AT_RENAME_* macros when __USE_GNU is defined since
> >> /usr/include/stdio.h defines them in that case (i.e. when _GNU_SOURCE
> >> is defined, which causes __USE_GNU to be defined).
> >>
> >> Having them defined in 2 places causes build warnings (duplicate
> >> definitions) in both samples/watch_queue/watch_test.c and
> >> samples/vfs/test-statx.c.
> >
> > It does?  What flags?
> >
>
> for samples/vfs/test-statx.c:
>
> In file included from ../samples/vfs/test-statx.c:23:
> usr/include/linux/fcntl.h:159:9: warning: ‘AT_RENAME_NOREPLACE’ redefined
>   159 | #define AT_RENAME_NOREPLACE     0x0001
> In file included from ../samples/vfs/test-statx.c:13:
> /usr/include/stdio.h:171:10: note: this is the location of the previous definition
>   171 | # define AT_RENAME_NOREPLACE RENAME_NOREPLACE
> usr/include/linux/fcntl.h:160:9: warning: ‘AT_RENAME_EXCHANGE’ redefined
>   160 | #define AT_RENAME_EXCHANGE      0x0002
> /usr/include/stdio.h:173:10: note: this is the location of the previous definition
>   173 | # define AT_RENAME_EXCHANGE RENAME_EXCHANGE
> usr/include/linux/fcntl.h:161:9: warning: ‘AT_RENAME_WHITEOUT’ redefined
>   161 | #define AT_RENAME_WHITEOUT      0x0004
> /usr/include/stdio.h:175:10: note: this is the location of the previous definition
>   175 | # define AT_RENAME_WHITEOUT RENAME_WHITEOUT
>
> for samples/watch_queue/watch_test.c:
>
> In file included from usr/include/linux/watch_queue.h:6,
>                  from ../samples/watch_queue/watch_test.c:19:
> usr/include/linux/fcntl.h:159:9: warning: ‘AT_RENAME_NOREPLACE’ redefined
>   159 | #define AT_RENAME_NOREPLACE     0x0001
> In file included from ../samples/watch_queue/watch_test.c:11:
> /usr/include/stdio.h:171:10: note: this is the location of the previous definition
>   171 | # define AT_RENAME_NOREPLACE RENAME_NOREPLACE
> usr/include/linux/fcntl.h:160:9: warning: ‘AT_RENAME_EXCHANGE’ redefined
>   160 | #define AT_RENAME_EXCHANGE      0x0002
> /usr/include/stdio.h:173:10: note: this is the location of the previous definition
>   173 | # define AT_RENAME_EXCHANGE RENAME_EXCHANGE
> usr/include/linux/fcntl.h:161:9: warning: ‘AT_RENAME_WHITEOUT’ redefined
>   161 | #define AT_RENAME_WHITEOUT      0x0004
> /usr/include/stdio.h:175:10: note: this is the location of the previous definition
>   175 | # define AT_RENAME_WHITEOUT RENAME_WHITEOUT
>
>
> > #define AT_RENAME_NOREPLACE     0x0001
> > #define AT_RENAME_NOREPLACE     0x0001
> >
> > int main(void)
> > {
> >       return AT_RENAME_NOREPLACE;
> > }
> >
> > gcc -W -Wall testA.c -o testA
> >
> > (no warnings)
> >
> > I'm pretty sure C says that duplicate definitions are fine as long
> > as they're identical.
> The vales are identical but the strings are not identical.
>
> We can't fix stdio.h, but we could just change uapi/linux/fcntl.h
> to match stdio.h. I suppose.

I do not specifically object to a patch like this (assuming that is works?):

--- a/include/uapi/linux/fcntl.h
+++ b/include/uapi/linux/fcntl.h
@@ -156,9 +156,9 @@
  */

 /* Flags for renameat2(2) (must match legacy RENAME_* flags). */
-#define AT_RENAME_NOREPLACE    0x0001
-#define AT_RENAME_EXCHANGE     0x0002
-#define AT_RENAME_WHITEOUT     0x0004
+#define AT_RENAME_NOREPLACE    RENAME_NOREPLACE
+#define AT_RENAME_EXCHANGE     RENAME_EXCHANGE
+#define AT_RENAME_WHITEOUT     RENAME_WHITEOUT


But to be clear, this is a regression introduced by glibc that is likely
to break many other builds, not only the kernel samples
and even if we fix linux uapi to conform to its downstream
copy of definitions, it won't help those users whose programs
build was broken until they install kernel headers, so feels like you
should report this regression to glibc and they'd better not "fix" the
regression by copying the current definition string as that may change as per
the patch above.

Why would a library copy definitions from kernel uapi without
wrapping them with #ifndef or #undef?

Thanks,
Amir.