Messages in this thread | | | From | Arnd Bergmann <> | Date | Mon, 10 Jan 2022 14:35:19 +0100 | Subject | Re: [PATCH V2 03/17] asm-generic: fcntl: compat: Remove duplicate definitions |
| |
On Tue, Dec 28, 2021 at 3:39 PM <guoren@kernel.org> wrote: > > From: Guo Ren <guoren@linux.alibaba.com> > > Remove duplicate F_GETLK64,F_SETLK64,F_SETLKW64 definitions in > arch/*/include/asm/compat.h. > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > Signed-off-by: Guo Ren <guoren@kernel.org> > Cc: Arnd Bergmann <arnd@arndb.de>
Unfortunately, this one does not look correct to me:
> @@ -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
| |