[LTP] [PATCH] syscalls/fcntl34: use struct flock64 on 32bit

[LTP] [PATCH] syscalls/fcntl34: use struct flock64 on 32bit

[Jiri Jaburek]

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.

Signed-off-by: Jiri Jaburek <jjaburek@redhat.com>
---
 testcases/kernel/syscalls/fcntl/fcntl34.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/fcntl/fcntl34.c b/testcases/kernel/syscalls/fcntl/fcntl34.c
index c72951e..9c9310e 100644
--- a/testcases/kernel/syscalls/fcntl/fcntl34.c
+++ b/testcases/kernel/syscalls/fcntl/fcntl34.c
@@ -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

[LTP] [PATCH] syscalls/fcntl34: use struct flock64 on 32bit

[Jiri Jaburek]

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.

[LTP] [PATCH v2] syscalls/fcntl34: disable on 32bit without _FILE_OFFSET_BITS==64

[Jiri Jaburek]

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.

Signed-off-by: Jiri Jaburek <jjaburek@redhat.com>
---
 testcases/kernel/syscalls/fcntl/fcntl34.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/testcases/kernel/syscalls/fcntl/fcntl34.c b/testcases/kernel/syscalls/fcntl/fcntl34.c
index c72951e..e2c8bf7 100644
--- a/testcases/kernel/syscalls/fcntl/fcntl34.c
+++ b/testcases/kernel/syscalls/fcntl/fcntl34.c
@@ -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

[LTP] [PATCH] syscalls/fcntl34: use struct flock64 on 32bit

[Cyril Hrubis]

Hi!
> 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.

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
chrubis@suse.cz

[LTP] [PATCH] syscalls/fcntl34: use struct flock64 on 32bit

[Jiri Jaburek]

On 06/13/16 16:22, Cyril Hrubis wrote:
> 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.

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.

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

[LTP] [PATCH] syscalls/fcntl34: use struct flock64 on 32bit

[Cyril Hrubis]

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.
> 
> That might not be possible as such "silent" conversion could break
> a lot of existing programs that rely on hardcoded 32bit struct flock
> size.

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
chrubis@suse.cz