Skip to contentSkip to conversation

fcntl-linux.h: add F_OFD_*32 constants

PATCH Superseded Jeff Layton 2 participants 4 messages Aug 18, 2016 – Aug 19, 2016 VFS

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fcntl-linux.h: add F_OFD_*32 constants

The original assumption with OFD lock support was that userland would always pass in a struct flock that has been set up for large file support (LFS). It's possible however for someone to build a 32-bit program without large file support, and still try to use OFD locks. If that happens then it will send a non-LFS struct flock to the kernel, which will then try to interpret it as a LFS one, which can cause unexpected results.

My original idea was to just ensure that the build would fail in this case by not defining the F_OFD_* constants when LFS support was not enabled.

Instead though, it's probably better long-term to just go ahead and support OFD locks with non-LFS struct flock. To do that without breaking programs that are already working, we add a set of F_OFD_*32 constants, and add the plumbing to the kernel to make those do the right thing.

This patch adds the same constants to glibc, and has it redefine the "normal" F_OFD_* constants to their *32 equvalents when LFS support is not in use.

---
 ChangeLog                                  |  4 ++++
 sysdeps/unix/sysv/linux/bits/fcntl-linux.h | 17 +++++++++++++----
 2 files changed, 17 insertions(+), 4 deletions(-)
ChangeLog +4 -0
index 59c68d8b9d0d..4a2488b50054 100644
@@ -1,3 +1,7 @@
+2016-08-18  Jeff Layton <[email protected]>
+	* sysdeps/unix/sysv/linux/bits/fcntl-linux.h:
+	Add F_OFD_GETLK32, F_OFD_SETLK32, F_OFD_SETLKW32
+
 2016-08-18  Stefan Liebler  <[email protected]>
 
 	* sysdeps/ieee754/dbl-64/k_rem_pio2.c (__kernel_rem_pio2):
sysdeps/unix/sysv/linux/bits/fcntl-linux.h +13 -4
index 7e5b0aecdcb4..9cb1b5f6f3ec 100644
@@ -127,11 +127,20 @@
    This means that they are inherited across fork or clone with CLONE_FILES
    like BSD (flock) locks, and they are only released automatically when the
    last reference to the the file description against which they were acquired
-   is put. */
+   is put.  */
 #ifdef __USE_GNU
-# define F_OFD_GETLK	36
-# define F_OFD_SETLK	37
-# define F_OFD_SETLKW	38
+# if __OFF_T_MATCHES_OFF64_T || defined __USE_FILE_OFFSET64
+#  define F_OFD_GETLK		36
+#  define F_OFD_SETLK		37
+#  define F_OFD_SETLKW		38
+# else
+#  define F_OFD_GETLK32		39
+#  define F_OFD_SETLK32		40
+#  define F_OFD_SETLKW32	41
+#  define F_OFD_GETLK		F_OFD_GETLK32
+#  define F_OFD_SETLK		F_OFD_SETLK32
+#  define F_OFD_SETLKW		F_OFD_SETLKW32
+# endif
 #endif
 
 #ifdef __USE_LARGEFILE64
Signature · 1 line
2.7.4
#1 lore raw

ChangeLog line 3

Quoted 3 lines · Jeff Layton

+2016-08-18 Jeff Layton <[email protected]>

ChangeLog +2 -0
+	* sysdeps/unix/sysv/linux/bits/fcntl-linux.h:
+	Add F_OFD_GETLK32, F_OFD_SETLK32, F_OFD_SETLKW32

Should be a blank line after the first one. look at all the other entries in this file as an example.

Quoted 15 lines
sysdeps/unix/sysv/linux/bits/fcntl-linux.h +4 -4
@@ -127,11 +127,20 @@
    This means that they are inherited across fork or clone with CLONE_FILES
    like BSD (flock) locks, and they are only released automatically when the
    last reference to the the file description against which they were acquired
-   is put. */
+   is put.  */
 #ifdef __USE_GNU
-# define F_OFD_GETLK	36
-# define F_OFD_SETLK	37
-# define F_OFD_SETLKW	38
+# if __OFF_T_MATCHES_OFF64_T || defined __USE_FILE_OFFSET64
+#  define F_OFD_GETLK		36
+#  define F_OFD_SETLK		37

sysdeps/unix/sysv/linux/bits/fcntl-linux.h line 144

Quoted 10 lines
sysdeps/unix/sysv/linux/bits/fcntl-linux.h +9 -0
+#  define F_OFD_SETLKW		38
+# else
+#  define F_OFD_GETLK32		39
+#  define F_OFD_SETLK32		40
+#  define F_OFD_SETLKW32	41
+#  define F_OFD_GETLK		F_OFD_GETLK32
+#  define F_OFD_SETLK		F_OFD_SETLK32
+#  define F_OFD_SETLKW		F_OFD_SETLKW32
+# endif
 #endif

i think we should define *64 and *32 variants all the time, and then route the F_OFD_GETLK/etc... to them based on compile mode. -mike

#2 lore raw
Quoted 9 lines · Mike Frysinger
Quoted 4 lines · Jeff Layton
Quoted 4 lines

ChangeLog line 3

Quoted 4 lines

+2016-08-18  Jeff Layton <[email protected]>

ChangeLog +2 -0
+	* sysdeps/unix/sysv/linux/bits/fcntl-linux.h:
+	Add F_OFD_GETLK32, F_OFD_SETLK32, F_OFD_SETLKW32

Should be a blank line after the first one.  look at all the other entries in this file as an example.

Ok, will fix...

Quoted 27 lines
Quoted 18 lines
sysdeps/unix/sysv/linux/bits/fcntl-linux.h +0 -0
@@ -127,11 +127,20 @@

    This means that they are inherited across fork or clone with CLONE_FILES like BSD (flock) locks, and they are only released automatically when the last reference to the the file description against which they were acquired -   is put. */
+   is put.  */
 #ifdef __USE_GNU

Quoted 3 lines

sysdeps/unix/sysv/linux/bits/fcntl-linux.h line 134

Quoted 3 lines
sysdeps/unix/sysv/linux/bits/fcntl-linux.h +0 -3
-# define F_OFD_GETLK	36
-# define F_OFD_SETLK	37
-# define F_OFD_SETLKW	38

+# if __OFF_T_MATCHES_OFF64_T || defined __USE_FILE_OFFSET64

Quoted 3 lines

sysdeps/unix/sysv/linux/bits/fcntl-linux.h line 135

Quoted 3 lines
sysdeps/unix/sysv/linux/bits/fcntl-linux.h +3 -0
+#  define F_OFD_GETLK		36
+#  define F_OFD_SETLK		37
+#  define F_OFD_SETLKW		38

+# else

Quoted 8 lines
Quoted 6 lines

sysdeps/unix/sysv/linux/bits/fcntl-linux.h line 142

Quoted 6 lines
sysdeps/unix/sysv/linux/bits/fcntl-linux.h +6 -0
+#  define F_OFD_GETLK32		39
+#  define F_OFD_SETLK32		40
+#  define F_OFD_SETLKW32	41
+#  define F_OFD_GETLK		F_OFD_GETLK32
+#  define F_OFD_SETLK		F_OFD_SETLK32
+#  define F_OFD_SETLKW		F_OFD_SETLKW32

+# endif
 #endif

Quoted 3 lines

i think we should define *64 and *32 variants all the time, and then route the F_OFD_GETLK/etc... to them based on compile mode. -mike

Sorry, I don't quite understand here. The whole point is that the existing F_OFD_* constants are already implicitly 64-bit. Why do we need separate constants postfixed with "64" that no one will ever use?

Signature · 1 line
Jeff Layton <[email protected]>
#3 lore raw
Quoted 28 lines · Jeff Layton
Quoted 27 lines · Mike Frysinger
Quoted 25 lines · Jeff Layton
sysdeps/unix/sysv/linux/bits/fcntl-linux.h +0 -0
@@ -127,11 +127,20 @@

    This means that they are inherited across fork or clone with CLONE_FILES like BSD (flock) locks, and they are only released automatically when the last reference to the the file description against which they were acquired -   is put. */
+   is put.  */
 #ifdef __USE_GNU

Quoted 3 lines

sysdeps/unix/sysv/linux/bits/fcntl-linux.h line 134

Quoted 3 lines
sysdeps/unix/sysv/linux/bits/fcntl-linux.h +0 -3
-# define F_OFD_GETLK	36
-# define F_OFD_SETLK	37
-# define F_OFD_SETLKW	38

+# if __OFF_T_MATCHES_OFF64_T || defined __USE_FILE_OFFSET64

Quoted 3 lines

sysdeps/unix/sysv/linux/bits/fcntl-linux.h line 135

Quoted 3 lines
sysdeps/unix/sysv/linux/bits/fcntl-linux.h +3 -0
+#  define F_OFD_GETLK		36
+#  define F_OFD_SETLK		37
+#  define F_OFD_SETLKW		38

+# else

Quoted 6 lines

sysdeps/unix/sysv/linux/bits/fcntl-linux.h line 142

Quoted 6 lines
sysdeps/unix/sysv/linux/bits/fcntl-linux.h +6 -0
+#  define F_OFD_GETLK32		39
+#  define F_OFD_SETLK32		40
+#  define F_OFD_SETLKW32	41
+#  define F_OFD_GETLK		F_OFD_GETLK32
+#  define F_OFD_SETLK		F_OFD_SETLK32
+#  define F_OFD_SETLKW		F_OFD_SETLKW32

+# endif
 #endif

Quoted 6 lines
Quoted 2 lines

i think we should define *64 and *32 variants all the time, and then route the F_OFD_GETLK/etc... to them based on compile mode.

Sorry, I don't quite understand here. The whole point is that the existing F_OFD_* constants are already implicitly 64-bit. Why do we need separate constants postfixed with "64" that no one will ever use?

you're making them not explicitly 64-bit when off_t!=off64_t. all the other commands in this file have used the convention:

	<cmd>: automatically 32-bit or 64-bit
	<cmd>64: always 64-bit

so the header would do:
#define F_OFD_GETLK64 36
#define F_OFD_SETLK64 37
#define F_OFD_SETLKW64 38
#if __OFF_T_MATCHES_OFF64_T || defined __USE_FILE_OFFSET64

# define F_OFD_GETLK F_OFD_GETLK64
# define F_OFD_SETLK F_OFD_SETLK64
# define F_OFD_SETLKW F_OFD_SETLKW64
#else
# define F_OFD_GETLK 39
# define F_OFD_SETLK 40
# define F_OFD_SETLKW 41
#endif

the defines deviate a bit from the names used on the kernel side, but we've already done that, and you're proposed patch does too. -mike