Re: [PATCH V2 03/17] asm-generic: fcntl: compat: Remove duplicate definitions
From: Arnd Bergmann <arnd@arndb.de>
Date: 2022-01-10 13:35:39
Also in:
linux-fsdevel, linux-mips, linux-riscv, linux-s390, linuxppc-dev, lkml, sparclinux
On Tue, Dec 28, 2021 at 3:39 PM [off-list ref] wrote:
From: Guo Ren <redacted> Remove duplicate F_GETLK64,F_SETLK64,F_SETLKW64 definitions in arch/*/include/asm/compat.h. Signed-off-by: Guo Ren <redacted> Signed-off-by: Guo Ren <guoren@kernel.org> Cc: Arnd Bergmann <arnd@arndb.de>
Unfortunately, this one does not look correct to me:
quoted hunk ↗ jump to hunk
@@ -116,7 +116,7 @@ #define F_GETSIG 11 /* for sockets. */ #endif -#ifndef CONFIG_64BIT +#if !defined(CONFIG_64BIT) || defined(CONFIG_COMPAT) #ifndef F_GETLK64 #define F_GETLK64 12 /* using 'struct flock64' */ #define F_SETLK64 13
The problem here is that include/uapi/ headers cannot contain checks for
CONFIG_* symbols because those may have different meanings in user space
compared to kernel.
This is a preexisting problem in the header, but I think the change
makes it worse.
With the current behavior, user space will always see the definitions,
unless it happens to have its own definition for CONFIG_64BIT already.
On 64-bit parisc, this has the effect of defining the macros to the
same values as F_SETOWN/F_SETSIG/F_GETSIG, which is potentially
harmful. On MIPS, it uses values that are different from the 32-bit numbers
but are otherwise unused. Everywhere else, we get the definition from
the 32-bit architecture in user space, which will do nothing in the kernel.
The correct check for a uapi header would be to test for
__BITS_PER_LONG==32. We should probably do that here, but
this won't help you move the definitions, and it is a user-visible change
as the incorrect definition will no longer be visible. [Adding Jeff and Bruce
(the flock mainainers) to Cc for additional feedback on this]
For your series, I would suggest just moving the macro definitions to
include/linux/compat.h along with the 'struct compat_flock64'
definition, and leaving the duplicate one in the uapi header unchanged
until we have decided on a solution.
Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel