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: <CAK8P3a3cuTknZaLZCFGwZtMfbd1qAFWEtXMcvVHsXoJn8EUCOg@mail.gmail.com>
Date:   Tue, 19 Jul 2022 09:42:23 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Christoph Hellwig <hch@....de>,
        Nathan Chancellor <nathan@...nel.org>,
        Naresh Kamboju <naresh.kamboju@...aro.org>,
        "open list:BROADCOM NVRAM DRIVER" <linux-mips@...r.kernel.org>,
        linux-arch <linux-arch@...r.kernel.org>,
        Heiko Stuebner <heiko@...ech.de>,
        Arnd Bergmann <arnd@...db.de>,
        Palmer Dabbelt <palmer@...osinc.com>,
        Guo Ren <guoren@...nel.org>
Subject: Re: [PATCH] tools: Fixed MIPS builds due to struct flock re-definition

On Fri, Jul 15, 2022 at 8:55 PM Florian Fainelli <f.fainelli@...il.com> wrote:
>
> Building perf for MIPS failed after 9f79b8b72339 ("uapi: simplify
> __ARCH_FLOCK{,64}_PAD a little") with the following error:
>
>   CC
> /home/fainelli/work/buildroot/output/bmips/build/linux-custom/tools/perf/trace/beauty/fcntl.o
> In file included from
> ../../../../host/mipsel-buildroot-linux-gnu/sysroot/usr/include/asm/fcntl.h:77,
>                  from ../include/uapi/linux/fcntl.h:5,
>                  from trace/beauty/fcntl.c:10:
> ../include/uapi/asm-generic/fcntl.h:188:8: error: redefinition of
> 'struct flock'
>  struct flock {
>         ^~~~~
> In file included from ../include/uapi/linux/fcntl.h:5,
>                  from trace/beauty/fcntl.c:10:
> ../../../../host/mipsel-buildroot-linux-gnu/sysroot/usr/include/asm/fcntl.h:63:8:
> note: originally defined here
>  struct flock {
>         ^~~~~
>
> This is due to the local copy under
> tools/include/uapi/asm-generic/fcntl.h including the toolchain's kernel
> headers which already define 'struct flock' and define
> HAVE_ARCH_STRUCT_FLOCK to future inclusions make a decision as to
> whether re-defining 'struct flock' is appropriate or not.
>
> Make sure what do not re-define 'struct flock'
> when HAVE_ARCH_STRUCT_FLOCK is already defined.
>
> Fixes: 9f79b8b72339 ("uapi: simplify __ARCH_FLOCK{,64}_PAD a little")
> Signed-off-by: Florian Fainelli <f.fainelli@...il.com>
> ---
>  tools/include/uapi/asm-generic/fcntl.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/tools/include/uapi/asm-generic/fcntl.h b/tools/include/uapi/asm-generic/fcntl.h
> index 0197042b7dfb..312881aa272b 100644
> --- a/tools/include/uapi/asm-generic/fcntl.h
> +++ b/tools/include/uapi/asm-generic/fcntl.h
> @@ -185,6 +185,7 @@ struct f_owner_ex {
>
>  #define F_LINUX_SPECIFIC_BASE  1024
>
> +#ifndef HAVE_ARCH_STRUCT_FLOCK
>  struct flock {
>         short   l_type;
>         short   l_whence;
> @@ -209,5 +210,6 @@ struct flock64 {
>         __ARCH_FLOCK64_PAD
>  #endif
>  };
> +#endif /* HAVE_ARCH_STRUCT_FLOCK */
>

I applied this to the asm-generic tree, but now I'm having second thoughts, as
this only changes the tools/include/ version but not the version we ship to user
space. Normally these are meant to be kept in sync.

It appears that commit 306f7cc1e906 ("uapi: always define
F_GETLK64/F_SETLK64/F_SETLKW64 in fcntl.h") already caused
them to diverge, presumably the uapi version here is correct and we
forgot to adapt the tools version at some point. There are also some
non-functional differences from older patches.

I think the correct fix to address the problem in both versions and
get them back into sync would be something like the patch below.
I have done zero testing on it though.

Christoph and Florian, any other suggestions?

       Arnd

diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index f13d37b60775..1ecdb911add8 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -192,6 +192,7 @@ struct f_owner_ex {

 #define F_LINUX_SPECIFIC_BASE  1024

+#ifndef HAVE_ARCH_STRUCT_FLOCK
 struct flock {
        short   l_type;
        short   l_whence;
@@ -216,5 +217,6 @@ struct flock64 {
        __ARCH_FLOCK64_PAD
 #endif
 };
+#endif /* HAVE_ARCH_STRUCT_FLOCK */

 #endif /* _ASM_GENERIC_FCNTL_H */
diff --git a/tools/include/uapi/asm-generic/fcntl.h
b/tools/include/uapi/asm-generic/fcntl.h
index 312881aa272b..1ecdb911add8 100644
--- a/tools/include/uapi/asm-generic/fcntl.h
+++ b/tools/include/uapi/asm-generic/fcntl.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
 #ifndef _ASM_GENERIC_FCNTL_H
 #define _ASM_GENERIC_FCNTL_H

@@ -90,7 +91,7 @@

 /* a horrid kludge trying to make sure that this will fail on old kernels */
 #define O_TMPFILE (__O_TMPFILE | O_DIRECTORY)
-#define O_TMPFILE_MASK (__O_TMPFILE | O_DIRECTORY | O_CREAT)
+#define O_TMPFILE_MASK (__O_TMPFILE | O_DIRECTORY | O_CREAT)

 #ifndef O_NDELAY
 #define O_NDELAY       O_NONBLOCK
@@ -115,11 +116,13 @@
 #define F_GETSIG       11      /* for sockets. */
 #endif

+#if __BITS_PER_LONG == 32 || defined(__KERNEL__)
 #ifndef F_GETLK64
 #define F_GETLK64      12      /*  using 'struct flock64' */
 #define F_SETLK64      13
 #define F_SETLKW64     14
 #endif
+#endif /* __BITS_PER_LONG == 32 || defined(__KERNEL__) */

 #ifndef F_SETOWN_EX
 #define F_SETOWN_EX    15
@@ -178,6 +181,10 @@ struct f_owner_ex {
                                   blocking */
 #define LOCK_UN                8       /* remove lock */

+/*
+ * LOCK_MAND support has been removed from the kernel. We leave the symbols
+ * here to not break legacy builds, but these should not be used in new code.
+ */
 #define LOCK_MAND      32      /* This is a mandatory flock ... */
 #define LOCK_READ      64      /* which allows concurrent read operations */
 #define LOCK_WRITE     128     /* which allows concurrent write operations */

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ