syscalls/fcntl34: use struct flock64 on 32bit

On x86, compiled as 32bit:

fcntl34.c:113: INFO: write to a file inside threads with OFD locks fcntl34.c:47: INFO: spawning '3' threads
fcntl34.c:56: INFO: waiting for '3' threads fcntl34.c:88: BROK: fcntl() failed: EINVAL

This is because glibc translates fcntl() to sys_fcntl64,

[pid 19656] fcntl64(4, F_OFD_SETLKW, {l_type=F_WRLCK, l_whence=SEEK_SET, l_start=4294967296, l_len=-695062700769673216}) = -1 EINVAL (Invalid argument)

as advised by the kernel,

	/* 32-bit arches must use fcntl64() */
	case F_OFD_SETLK:
	case F_OFD_SETLKW:

but the struct passed uses 32bit values,

struct flock lck = {
	.l_whence = SEEK_SET,
	.l_start = 0,
	.l_len = 1,
};

which are read as 64bit by the kernel,

int fcntl_setlk64(unsigned int fd, struct file *filp, unsigned int cmd, struct flock64 __user *l)

causing the garbage in upper 32bit of at least l_start and l_len to be treated as offsets, resulting in EINVAL.

This patch makes the test use struct flock64 when _FILE_OFFSET_BITS is unset.

---
 testcases/kernel/syscalls/fcntl/fcntl34.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

index c72951e..9c9310e 100644
@@ -58,6 +58,13 @@ static void wait_threads(pthread_t *id)
 		SAFE_PTHREAD_JOIN(id[i], NULL);
 }
 
+/* F_OFD_SETLKW needs proper 64bit offsets on 32bit systems */
+#if _FILE_OFFSET_BITS == 64
+typedef struct flock flock_t;
+#else
+typedef struct flock64 flock_t;
+#endif
+
 void *thread_fn_01(void *arg)
 {
 	int i;

@@ -66,7 +73,7 @@ void *thread_fn_01(void *arg)
 
 	memset(buf, (intptr_t)arg, write_size);
 
-	struct flock lck = {
+	flock_t lck = {
 		.l_whence = SEEK_SET,
 		.l_start  = 0,
 		.l_len    = 1,

2.4.3

On a second thought, the patched code isn't very nice on 64bit systems (even though the sizes work out correctly). Also, the same codepath is tested (on 32bit) via the _64 build, so it would perhaps make more sense to disable the non-FILE_OFFSET_BITS=64 test on 32bit.

I'll send a new patch.

Thanks.

On x86, compiled as 32bit:

fcntl34.c:113: INFO: write to a file inside threads with OFD locks fcntl34.c:47: INFO: spawning '3' threads
fcntl34.c:56: INFO: waiting for '3' threads fcntl34.c:88: BROK: fcntl() failed: EINVAL

This is because glibc translates fcntl() to sys_fcntl64,

[pid 19656] fcntl64(4, F_OFD_SETLKW, {l_type=F_WRLCK, l_whence=SEEK_SET, l_start=4294967296, l_len=-695062700769673216}) = -1 EINVAL (Invalid argument)

as advised by the kernel,

	/* 32-bit arches must use fcntl64() */
	case F_OFD_SETLK:
	case F_OFD_SETLKW:

but the struct passed uses 32bit values, which are read as 64bit by the kernel,

SYSCALL_DEFINE3(fcntl64, unsigned int, fd, unsigned int, cmd,
unsigned long, arg)

{
	...
	case F_OFD_SETLKW:
		err = fcntl_setlk64(fd, f.file, cmd,
				(struct flock64 __user *) arg);

causing the garbage in upper 32bit of at least l_start and l_len to be treated as offsets, resulting in EINVAL.

This patch therefore disables the test when using the 32bit struct members. The functionality is tested by fcntl34_64, which defines _FILE_OFFSET_BITS=64.

---
 testcases/kernel/syscalls/fcntl/fcntl34.c | 4 ++++
 1 file changed, 4 insertions(+)

index c72951e..e2c8bf7 100644
@@ -102,6 +102,10 @@ static void test01(void)
 
 	tst_res(TINFO, "write to a file inside threads with OFD locks");
 
+#if __WORDSIZE == 32 && _FILE_OFFSET_BITS != 64
+	tst_brk(TCONF, "test not supported on 32bit");
+#endif
+
 	int fd = SAFE_OPEN(fname, O_CREAT | O_TRUNC | O_RDWR, 0600);
 
 	memset(res, 0, sizeof(res));

2.4.3

Hi!

Since both manual and glibc examples use struct flock with OFD locks in examples and if I compile glibc example for OFD locks[1] on 32bit system garbage is passed to kernel syscalls and the program hangs, so I would call this glibc/kernel bug.

I would expect glibc to convert the flock structure to 64 bit one silently in this case.

[1] manual/examples/ofdlocks.c

    https://sourceware.org/git/?p=glibc.git;a=blob;f=manual/examples/ofdlocks.c;h=ba4f0ef4d237e95b8f1e0f37b9c1befd4afda0d4;hb=HEAD

Cyril Hrubis
[email protected]

That might not be possible as such "silent" conversion could break a lot of existing programs that rely on hardcoded 32bit struct flock size.

However I double-tested it with systemtap and filed the bug anyway,
https://sourceware.org/bugzilla/show_bug.cgi?id=20251

Thanks for pointing out the glibc example.

Hi!

What I meant was to use the user supplied flock structure to initialize flock64 before doing the syscall. We would have to copy the l_type back in case of FGETLK but apart from that I do not see any problem with this.

Also I've tried to run the test on purely 32bit machine (i686 openSUSE Tumbleweed) and the problem seems to be present there as well. I can see that it calls fcntl64() which fails with EINVAL and both l_start and l_len are clearly garbage. So 32bit glibc with 32bit kernel seems to be broken as well.

Cyril Hrubis
[email protected]