2021-04-12 09:18:32

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 4/5] uapi: always define F_GETLK64/F_SETLK64/F_SETLKW64 in fcntl.h

The F_GETLK64/F_SETLK64/F_SETLKW64 commands are only implemented for
32-bit syscall APIs, but we also need them for compat handling on 64-bit
kernels.

Given that redefining them is rather error prone, as shown by parisc
getting the opcodes wrong currently, just use the existing definitions
for the compat handling.

In theory we could try to hide them from userspace; but given that only
MIPS manages to properly do that while the asm-generic version used by
most architectures relies on a Kconfig symbol that is never set by
userspace there doesn't seem to be much of a point to even bother.

Signed-off-by: Christoph Hellwig <[email protected]>
---
arch/arm64/include/asm/compat.h | 4 ----
arch/mips/include/asm/compat.h | 4 ----
arch/mips/include/uapi/asm/fcntl.h | 2 --
arch/powerpc/include/asm/compat.h | 4 ----
arch/s390/include/asm/compat.h | 4 ----
arch/sparc/include/asm/compat.h | 4 ----
arch/x86/include/asm/compat.h | 4 ----
include/uapi/asm-generic/fcntl.h | 2 --
tools/include/uapi/asm-generic/fcntl.h | 2 --
9 files changed, 30 deletions(-)

diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
index 23a9fb73c04ff8..a5fe4558a6ecc0 100644
--- a/arch/arm64/include/asm/compat.h
+++ b/arch/arm64/include/asm/compat.h
@@ -74,10 +74,6 @@ struct compat_flock {
compat_pid_t l_pid;
};

-#define F_GETLK64 12 /* using 'struct flock64' */
-#define F_SETLK64 13
-#define F_SETLKW64 14
-
struct compat_flock64 {
short l_type;
short l_whence;
diff --git a/arch/mips/include/asm/compat.h b/arch/mips/include/asm/compat.h
index 65975712a22dcf..a13436e91c3938 100644
--- a/arch/mips/include/asm/compat.h
+++ b/arch/mips/include/asm/compat.h
@@ -60,10 +60,6 @@ struct compat_flock {
s32 pad[4];
};

-#define F_GETLK64 33
-#define F_SETLK64 34
-#define F_SETLKW64 35
-
struct compat_flock64 {
short l_type;
short l_whence;
diff --git a/arch/mips/include/uapi/asm/fcntl.h b/arch/mips/include/uapi/asm/fcntl.h
index 9e44ac810db94d..1769fc50d35f0e 100644
--- a/arch/mips/include/uapi/asm/fcntl.h
+++ b/arch/mips/include/uapi/asm/fcntl.h
@@ -44,11 +44,9 @@
#define F_SETOWN 24 /* for sockets. */
#define F_GETOWN 23 /* for sockets. */

-#ifndef __mips64
#define F_GETLK64 33 /* using 'struct flock64' */
#define F_SETLK64 34
#define F_SETLKW64 35
-#endif

#if _MIPS_SIM != _MIPS_SIM_ABI64
#define __ARCH_FLOCK_EXTRA_SYSID long l_sysid;
diff --git a/arch/powerpc/include/asm/compat.h b/arch/powerpc/include/asm/compat.h
index 9191fc29e6ed11..b0f2c3f7fe45a5 100644
--- a/arch/powerpc/include/asm/compat.h
+++ b/arch/powerpc/include/asm/compat.h
@@ -56,10 +56,6 @@ struct compat_flock {
compat_pid_t l_pid;
};

-#define F_GETLK64 12 /* using 'struct flock64' */
-#define F_SETLK64 13
-#define F_SETLKW64 14
-
struct compat_flock64 {
short l_type;
short l_whence;
diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h
index ea5b9c34b7be5b..e0896758779da4 100644
--- a/arch/s390/include/asm/compat.h
+++ b/arch/s390/include/asm/compat.h
@@ -111,10 +111,6 @@ struct compat_flock {
compat_pid_t l_pid;
};

-#define F_GETLK64 12
-#define F_SETLK64 13
-#define F_SETLKW64 14
-
struct compat_flock64 {
short l_type;
short l_whence;
diff --git a/arch/sparc/include/asm/compat.h b/arch/sparc/include/asm/compat.h
index b85842cda99fe0..4524997424043f 100644
--- a/arch/sparc/include/asm/compat.h
+++ b/arch/sparc/include/asm/compat.h
@@ -85,10 +85,6 @@ struct compat_flock {
short __unused;
};

-#define F_GETLK64 12
-#define F_SETLK64 13
-#define F_SETLKW64 14
-
struct compat_flock64 {
short l_type;
short l_whence;
diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
index be09c7eac89f09..b1691cf148be10 100644
--- a/arch/x86/include/asm/compat.h
+++ b/arch/x86/include/asm/compat.h
@@ -59,10 +59,6 @@ struct compat_flock {
compat_pid_t l_pid;
};

-#define F_GETLK64 12 /* using 'struct flock64' */
-#define F_SETLK64 13
-#define F_SETLKW64 14
-
/*
* IA32 uses 4 byte alignment for 64 bit quantities,
* so we need to pack this structure.
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index 7e714443a8d2e3..6430a2f8023fc3 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -116,13 +116,11 @@
#define F_GETSIG 11 /* for sockets. */
#endif

-#ifndef CONFIG_64BIT
#ifndef F_GETLK64
#define F_GETLK64 12 /* using 'struct flock64' */
#define F_SETLK64 13
#define F_SETLKW64 14
#endif
-#endif

#ifndef F_SETOWN_EX
#define F_SETOWN_EX 15
diff --git a/tools/include/uapi/asm-generic/fcntl.h b/tools/include/uapi/asm-generic/fcntl.h
index bf961a71802e0e..6e16722026f39a 100644
--- a/tools/include/uapi/asm-generic/fcntl.h
+++ b/tools/include/uapi/asm-generic/fcntl.h
@@ -115,13 +115,11 @@
#define F_GETSIG 11 /* for sockets. */
#endif

-#ifndef CONFIG_64BIT
#ifndef F_GETLK64
#define F_GETLK64 12 /* using 'struct flock64' */
#define F_SETLK64 13
#define F_SETLKW64 14
#endif
-#endif

#ifndef F_SETOWN_EX
#define F_SETOWN_EX 15
--
2.30.1


2021-04-13 21:20:39

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 4/5] uapi: always define F_GETLK64/F_SETLK64/F_SETLKW64 in fcntl.h

On Tue, Apr 13, 2021 at 05:43:18PM +0200, Helge Deller wrote:
> On 4/12/21 10:55 AM, Christoph Hellwig wrote:
>> The F_GETLK64/F_SETLK64/F_SETLKW64 commands are only implemented for
>> 32-bit syscall APIs, but we also need them for compat handling on 64-bit
>> kernels.
>
> Ok.
>
>> Given that redefining them is rather error prone, as shown by parisc
>> getting the opcodes wrong currently, just use the existing definitions
>> for the compat handling.
>
> Can you please show me where parisc gets it currently wrong?
> I'm somehow blind to see it and of course I'd like to fix it in
> stable kernel series if that's true...

Looking again I think my comment was wrong, I thought parisc would
fall back to the asm-generic version with different code points for
the compat case, but parisc actually already defines the constants
unconditionally like this patch does for everyone else. So I'll
retract that part of the comment.

2021-04-13 21:25:51

by Helge Deller

[permalink] [raw]
Subject: Re: [PATCH 4/5] uapi: always define F_GETLK64/F_SETLK64/F_SETLKW64 in fcntl.h

On 4/13/21 5:46 PM, Christoph Hellwig wrote:
> On Tue, Apr 13, 2021 at 05:43:18PM +0200, Helge Deller wrote:
>> On 4/12/21 10:55 AM, Christoph Hellwig wrote:
>>> The F_GETLK64/F_SETLK64/F_SETLKW64 commands are only implemented for
>>> 32-bit syscall APIs, but we also need them for compat handling on 64-bit
>>> kernels.
>>
>> Ok.
>>
>>> Given that redefining them is rather error prone, as shown by parisc
>>> getting the opcodes wrong currently, just use the existing definitions
>>> for the compat handling.
>>
>> Can you please show me where parisc gets it currently wrong?
>> I'm somehow blind to see it and of course I'd like to fix it in
>> stable kernel series if that's true...
>
> Looking again I think my comment was wrong, I thought parisc would
> fall back to the asm-generic version with different code points for
> the compat case, but parisc actually already defines the constants
> unconditionally like this patch does for everyone else. So I'll
> retract that part of the comment.

Ok, good.
Thanks for checking!

Helge

2021-04-14 02:07:12

by Helge Deller

[permalink] [raw]
Subject: Re: [PATCH 4/5] uapi: always define F_GETLK64/F_SETLK64/F_SETLKW64 in fcntl.h

On 4/12/21 10:55 AM, Christoph Hellwig wrote:
> The F_GETLK64/F_SETLK64/F_SETLKW64 commands are only implemented for
> 32-bit syscall APIs, but we also need them for compat handling on 64-bit
> kernels.

Ok.

> Given that redefining them is rather error prone, as shown by parisc
> getting the opcodes wrong currently, just use the existing definitions
> for the compat handling.

Can you please show me where parisc gets it currently wrong?
I'm somehow blind to see it and of course I'd like to fix it in
stable kernel series if that's true...

Other than that, thanks for this cleanup!
You may add:

Acked-by: Helge Deller <[email protected]>

Thanks!
Helge


> In theory we could try to hide them from userspace; but given that only
> MIPS manages to properly do that while the asm-generic version used by
> most architectures relies on a Kconfig symbol that is never set by
> userspace there doesn't seem to be much of a point to even bother.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> arch/arm64/include/asm/compat.h | 4 ----
> arch/mips/include/asm/compat.h | 4 ----
> arch/mips/include/uapi/asm/fcntl.h | 2 --
> arch/powerpc/include/asm/compat.h | 4 ----
> arch/s390/include/asm/compat.h | 4 ----
> arch/sparc/include/asm/compat.h | 4 ----
> arch/x86/include/asm/compat.h | 4 ----
> include/uapi/asm-generic/fcntl.h | 2 --
> tools/include/uapi/asm-generic/fcntl.h | 2 --
> 9 files changed, 30 deletions(-)
>
> diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
> index 23a9fb73c04ff8..a5fe4558a6ecc0 100644
> --- a/arch/arm64/include/asm/compat.h
> +++ b/arch/arm64/include/asm/compat.h
> @@ -74,10 +74,6 @@ struct compat_flock {
> compat_pid_t l_pid;
> };
>
> -#define F_GETLK64 12 /* using 'struct flock64' */
> -#define F_SETLK64 13
> -#define F_SETLKW64 14
> -
> struct compat_flock64 {
> short l_type;
> short l_whence;
> diff --git a/arch/mips/include/asm/compat.h b/arch/mips/include/asm/compat.h
> index 65975712a22dcf..a13436e91c3938 100644
> --- a/arch/mips/include/asm/compat.h
> +++ b/arch/mips/include/asm/compat.h
> @@ -60,10 +60,6 @@ struct compat_flock {
> s32 pad[4];
> };
>
> -#define F_GETLK64 33
> -#define F_SETLK64 34
> -#define F_SETLKW64 35
> -
> struct compat_flock64 {
> short l_type;
> short l_whence;
> diff --git a/arch/mips/include/uapi/asm/fcntl.h b/arch/mips/include/uapi/asm/fcntl.h
> index 9e44ac810db94d..1769fc50d35f0e 100644
> --- a/arch/mips/include/uapi/asm/fcntl.h
> +++ b/arch/mips/include/uapi/asm/fcntl.h
> @@ -44,11 +44,9 @@
> #define F_SETOWN 24 /* for sockets. */
> #define F_GETOWN 23 /* for sockets. */
>
> -#ifndef __mips64
> #define F_GETLK64 33 /* using 'struct flock64' */
> #define F_SETLK64 34
> #define F_SETLKW64 35
> -#endif
>
> #if _MIPS_SIM != _MIPS_SIM_ABI64
> #define __ARCH_FLOCK_EXTRA_SYSID long l_sysid;
> diff --git a/arch/powerpc/include/asm/compat.h b/arch/powerpc/include/asm/compat.h
> index 9191fc29e6ed11..b0f2c3f7fe45a5 100644
> --- a/arch/powerpc/include/asm/compat.h
> +++ b/arch/powerpc/include/asm/compat.h
> @@ -56,10 +56,6 @@ struct compat_flock {
> compat_pid_t l_pid;
> };
>
> -#define F_GETLK64 12 /* using 'struct flock64' */
> -#define F_SETLK64 13
> -#define F_SETLKW64 14
> -
> struct compat_flock64 {
> short l_type;
> short l_whence;
> diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h
> index ea5b9c34b7be5b..e0896758779da4 100644
> --- a/arch/s390/include/asm/compat.h
> +++ b/arch/s390/include/asm/compat.h
> @@ -111,10 +111,6 @@ struct compat_flock {
> compat_pid_t l_pid;
> };
>
> -#define F_GETLK64 12
> -#define F_SETLK64 13
> -#define F_SETLKW64 14
> -
> struct compat_flock64 {
> short l_type;
> short l_whence;
> diff --git a/arch/sparc/include/asm/compat.h b/arch/sparc/include/asm/compat.h
> index b85842cda99fe0..4524997424043f 100644
> --- a/arch/sparc/include/asm/compat.h
> +++ b/arch/sparc/include/asm/compat.h
> @@ -85,10 +85,6 @@ struct compat_flock {
> short __unused;
> };
>
> -#define F_GETLK64 12
> -#define F_SETLK64 13
> -#define F_SETLKW64 14
> -
> struct compat_flock64 {
> short l_type;
> short l_whence;
> diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
> index be09c7eac89f09..b1691cf148be10 100644
> --- a/arch/x86/include/asm/compat.h
> +++ b/arch/x86/include/asm/compat.h
> @@ -59,10 +59,6 @@ struct compat_flock {
> compat_pid_t l_pid;
> };
>
> -#define F_GETLK64 12 /* using 'struct flock64' */
> -#define F_SETLK64 13
> -#define F_SETLKW64 14
> -
> /*
> * IA32 uses 4 byte alignment for 64 bit quantities,
> * so we need to pack this structure.
> diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
> index 7e714443a8d2e3..6430a2f8023fc3 100644
> --- a/include/uapi/asm-generic/fcntl.h
> +++ b/include/uapi/asm-generic/fcntl.h
> @@ -116,13 +116,11 @@
> #define F_GETSIG 11 /* for sockets. */
> #endif
>
> -#ifndef CONFIG_64BIT
> #ifndef F_GETLK64
> #define F_GETLK64 12 /* using 'struct flock64' */
> #define F_SETLK64 13
> #define F_SETLKW64 14
> #endif
> -#endif
>
> #ifndef F_SETOWN_EX
> #define F_SETOWN_EX 15
> diff --git a/tools/include/uapi/asm-generic/fcntl.h b/tools/include/uapi/asm-generic/fcntl.h
> index bf961a71802e0e..6e16722026f39a 100644
> --- a/tools/include/uapi/asm-generic/fcntl.h
> +++ b/tools/include/uapi/asm-generic/fcntl.h
> @@ -115,13 +115,11 @@
> #define F_GETSIG 11 /* for sockets. */
> #endif
>
> -#ifndef CONFIG_64BIT
> #ifndef F_GETLK64
> #define F_GETLK64 12 /* using 'struct flock64' */
> #define F_SETLK64 13
> #define F_SETLKW64 14
> #endif
> -#endif
>
> #ifndef F_SETOWN_EX
> #define F_SETOWN_EX 15
>