[LTP] [PATCH] syscalls/fcntl34: Use -D_FILE_OFFSET_BITS=64 for all cases

[LTP] [PATCH] syscalls/fcntl34: Use -D_FILE_OFFSET_BITS=64 for all cases

[Yuriy Kolerov]

This test uses OFD locks which are supported only by 64-bit ABI.
Thus OFD locks must be used with flock64 structure for sure in this
test. It is necessary because on some 32-bit targets (e.g. 32-bit
uClibc) flock is not mapped to flock64 when _FILE_OFFSET_BITS is not
set to 64. Thus fcntl34 and fcntl34_64 must be identical. It is an
ugly but the easiest solution.

Signed-off-by: Yuriy Kolerov <yuriy.kolerov@synopsys.com>
---
 testcases/kernel/syscalls/fcntl/Makefile | 1 +
 1 file changed, 1 insertion(+)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-syscalls-fcntl34-Use-D_FILE_OFFSET_BITS-64-for-all-c.patch
Type: text/x-patch
Size: 446 bytes
Desc: not available
URL: <http://lists.linux.it/pipermail/ltp/attachments/20160812/373b6df2/attachment.bin>

[LTP] [PATCH] syscalls/fcntl34: Use -D_FILE_OFFSET_BITS=64 for all cases

[Cyril Hrubis]

Hi!
> This test uses OFD locks which are supported only by 64-bit ABI.
> Thus OFD locks must be used with flock64 structure for sure in this
> test. It is necessary because on some 32-bit targets (e.g. 32-bit
> uClibc) flock is not mapped to flock64 when _FILE_OFFSET_BITS is not
> set to 64. Thus fcntl34 and fcntl34_64 must be identical. It is an
> ugly but the easiest solution.

Indeed the test fails on 32bit platform without the
_FILE_OFFSET_BITS=64. But it fails correctly with EINVAL in this case
since the ioctl() in question is not implemented.

So better solution would be to try the ioctl() in the test setup and
report TCONF if we got EINVAL.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] syscalls/fcntl34: Use -D_FILE_OFFSET_BITS=64 for all cases

[Yuriy Kolerov]

Hi Cyril,

As far as I know it fails in this case because fcntl always expects flock64 for OFD locks but gets 32-bit flock structure with non-zero l_pid field (OFD expects that this field contains zero otherwise returns EINVAL). The problem is that there are no 32-bit OFD locks in the kernel. So there are 2 options:

1. Set -D_FILE_OFFSET_BITS=64 to use flock64 by default.
2. Use flock64 and fcntl64 everywhere explicitly.

> -----Original Message-----
> From: Cyril Hrubis [mailto:chrubis@suse.cz]
> Sent: Monday, August 15, 2016 3:08 PM
> To: Yuriy Kolerov <yuriy.kolerov@synopsys.com>
> Cc: ltp@lists.linux.it; Vineet.Gupta1@synopsys.com;
> Alexey.Brodkin@synopsys.com
> Subject: Re: [LTP] [PATCH] syscalls/fcntl34: Use -D_FILE_OFFSET_BITS=64 for
> all cases
> 
> Hi!
> > This test uses OFD locks which are supported only by 64-bit ABI.
> > Thus OFD locks must be used with flock64 structure for sure in this
> > test. It is necessary because on some 32-bit targets (e.g. 32-bit
> > uClibc) flock is not mapped to flock64 when _FILE_OFFSET_BITS is not
> > set to 64. Thus fcntl34 and fcntl34_64 must be identical. It is an
> > ugly but the easiest solution.
> 
> Indeed the test fails on 32bit platform without the _FILE_OFFSET_BITS=64.
> But it fails correctly with EINVAL in this case since the ioctl() in question is not
> implemented.
> 
> So better solution would be to try the ioctl() in the test setup and report
> TCONF if we got EINVAL.
> 
> --
> Cyril Hrubis
> chrubis@suse.cz

[LTP] [PATCH] syscalls/fcntl34: Use -D_FILE_OFFSET_BITS=64 for all cases

[Cyril Hrubis]

Hi!
> As far as I know it fails in this case because fcntl always expects
> flock64 for OFD locks but gets 32-bit flock structure with non-zero
> l_pid field (OFD expects that this field contains zero otherwise
> returns EINVAL). The problem is that there are no 32-bit OFD locks in
> the kernel. So there are 2 options:

If you look into fs/fcntl.c in Linux kernel the F_OFD_XXX constants are
inside #if BITS_PER_LONG != 32 in the do_fcntl(). Hence 32bit fcntl()
returns EINVAL since these constants are missing from the switch() when
compiled on 32bit platform.

And the same apply for when you run 32bit binary on 64bit kernel. In
this case the fcntl syscall from fs/compat.c just returns EINVAL if the
cmd is F_OFD_XXX.

Which works fine for glibc, since if you compile without
_FILE_OFFSET_BITS=64 on 32bit it calls 32bit fcntl() syscall that
rightfully returns EINVAL and if you compile with _FILE_OFFSET_BITS=64
both flock structure and fcntl() are 64bit and everything works fine.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] syscalls/fcntl34: Use -D_FILE_OFFSET_BITS=64 for all cases

[Yuriy Kolerov]

Actually both glibc and uClibc use fcntl64 by default and usually fail when you try to pass 32-bit flock structure to it when you use OFD locks. Then fcntl64 fails in fcntl_setlk64 routing while checking l_pid field. I checked this case on 32-bit targets: x86 and ARC.

So fcntl64 is used by default. When you don't pass -D_FILE_OFFSET_BITS=64 then fcntl64 accidently uses 32-bit flock for 64-bit OFD and l_pid field may be non-zero. Such usage of API is wrong because may lead to undefined behavior (kernel tries to read l_pid from nowhere and sometimes it hangs Linux on ARC target).

Here is a small example:

---------------- 8< ----------------
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
#include <fcntl.h>
#include <stdio.h>
#include <errno.h>

#define F_OFD_SETLKW 38

int main()
{
	char fname[] = "ofdlock.txt";
	int fd = open(fname, O_CREAT | O_RDWR, 0666);

	if (fd == -1)
		perror("cannot open file");

	struct flock64 lck = {
		.l_type   = F_WRLCK,
		.l_whence = SEEK_SET,
		.l_start  = 0,
		.l_len    = 1,
		.l_pid    = 0,
	};

	if (fcntl(fd, F_OFD_SETLKW, &lck) == -1)
		perror("cannot lock");

	return 0;
}
---------------- 8< ----------------

Command line:
$ gcc -m32 ofdlock.c -o ofdlock-x86
$ ./ofdlock-x86

It works fine because we use flock64 explicitly. fcntl is always 64-bit. But if you change flock64 to flock you get an error:
$ ./ofdlock-x86 
cannot lock: Invalid argument

And everything works fine again if you pass -D_FILE_OFFSET_BITS=64:
$ gcc -m32 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 ofdlock.c -o ofdlock-x86
$ ./ofdlock-x86

> -----Original Message-----
> From: Cyril Hrubis [mailto:chrubis@suse.cz]
> Sent: Tuesday, August 16, 2016 1:02 PM
> To: Yuriy Kolerov <Yuriy.Kolerov@synopsys.com>
> Cc: ltp@lists.linux.it; Vineet.Gupta1@synopsys.com;
> Alexey.Brodkin@synopsys.com
> Subject: Re: [LTP] [PATCH] syscalls/fcntl34: Use -D_FILE_OFFSET_BITS=64 for
> all cases
> 
> Hi!
> > As far as I know it fails in this case because fcntl always expects
> > flock64 for OFD locks but gets 32-bit flock structure with non-zero
> > l_pid field (OFD expects that this field contains zero otherwise
> > returns EINVAL). The problem is that there are no 32-bit OFD locks in
> > the kernel. So there are 2 options:
> 
> If you look into fs/fcntl.c in Linux kernel the F_OFD_XXX constants are inside
> #if BITS_PER_LONG != 32 in the do_fcntl(). Hence 32bit fcntl() returns EINVAL
> since these constants are missing from the switch() when compiled on 32bit
> platform.
> 
> And the same apply for when you run 32bit binary on 64bit kernel. In this
> case the fcntl syscall from fs/compat.c just returns EINVAL if the cmd is
> F_OFD_XXX.
> 
> Which works fine for glibc, since if you compile without
> _FILE_OFFSET_BITS=64 on 32bit it calls 32bit fcntl() syscall that rightfully
> returns EINVAL and if you compile with _FILE_OFFSET_BITS=64 both flock
> structure and fcntl() are 64bit and everything works fine.
> 
> --
> Cyril Hrubis
> chrubis@suse.cz

[LTP] [PATCH] syscalls/fcntl34: Use -D_FILE_OFFSET_BITS=64 for all cases

[Peter Maydell]

On 16 August 2016 at 11:47, Yuriy Kolerov <Yuriy.Kolerov@synopsys.com> wrote:
> Actually both glibc and uClibc use fcntl64 by default and usually
> fail when you try to pass 32-bit flock structure to it when you
> use OFD locks. Then fcntl64 fails in fcntl_setlk64 routing while
> checking l_pid field.

Given the cleverness glibc applies to flock(), it might be more
useful for the LTP syscall tests to directly test the system calls
avoiding the libc wrappers. That way you could know that you've
definitely tested both flock and flock64 syscalls on platforms
where they exist.

thanks
-- PMM

[LTP] [PATCH] syscalls/fcntl34: Use -D_FILE_OFFSET_BITS=64 for all cases

[Cyril Hrubis]

Hi!
> > Actually both glibc and uClibc use fcntl64 by default and usually
> > fail when you try to pass 32-bit flock structure to it when you
> > use OFD locks. Then fcntl64 fails in fcntl_setlk64 routing while
> > checking l_pid field.
> 
> Given the cleverness glibc applies to flock(), it might be more
> useful for the LTP syscall tests to directly test the system calls
> avoiding the libc wrappers. That way you could know that you've
> definitely tested both flock and flock64 syscalls on platforms
> where they exist.

I guess that we should be testing both. As it stands now we do not test
the 32bit fcntl() at all.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] syscalls/fcntl34: Use -D_FILE_OFFSET_BITS=64 for all cases

[Cyril Hrubis]

Hi!
> Actually both glibc and uClibc use fcntl64 by default and usually fail
> when you try to pass 32-bit flock structure to it when you use OFD
> locks. Then fcntl64 fails in fcntl_setlk64 routing while checking
> l_pid field. I checked this case on 32-bit targets: x86 and ARC.

Ah, I've missed that.


Given that the manual for fcntl() says:

  The newer system call employs a different structure for file locking,
  flock64, and corresponding commands, F_GETLK64, F_SETLK64, and
  F_SETLKW64.

I guess that the F_OFD_XXX locks should be added to this list as they
explicitly require flock64 as well. And the fix to the test would be
using struct flock64 instead of struct flock.

-- 
Cyril Hrubis
chrubis@suse.cz