[v4] Fix Linux fcntl OFD locks for non-LFS architectures (BZ#20251)
Commit Message
Changes from previous version:
- Add a testcase for compat fcntl using OFD locks.
---
This patch fixes the OFD ("file private") locks for architectures that
support non-LFS flock definition (__USE_FILE_OFFSET64 not defined). The
issue in this case is both F_OFD_{GETLK,SETLK,SETLKW} and
F_{SET,GET}L{W}K64 expects a flock64 argument and when using old
F_OFD_* flags with a non LFS flock argument the kernel might interpret
the underlying data wrongly. Kernel idea originally was to avoid using
such flags in non-LFS syscall, but since GLIBC uses fcntl with LFS
semantic as default it is possible to provide the functionality and
avoid the bogus struct kernel passing by adjusting the struct manually
for the required flags.
The idea follows other LFS interfaces that provide two symbols:
1. A new LFS fcntl64 is added on default ABI with the usual macros to
select it for FILE_OFFSET_BITS=64.
2. The Linux non-LFS fcntl use a stack allocated struct flock64 for
F_OFD_{GETLK,SETLK,SETLKW} copy the results on the user provided
struct.
3. Keep a compat symbol with old broken semantic for architectures
that do not define __OFF_T_MATCHES_OFF64_T.
So for architectures which defines __USE_FILE_OFFSET64, fcntl64 will
aliased to fcntl and no adjustment would be required. So to actually
use F_OFD_* with LFS support the source must be built with LFS support
(_FILE_OFFSET_BITS=64).
Also F_OFD_SETLKW command is handled a cancellation point, as for
F_SETLKW{64}.
Checked on x86_64-linux-gnu and i686-linux-gnu.
[BZ #20251]
* NEWS: Mention fcntl64 addition.
* csu/check_fds.c: Replace __fcntl_nocancel by __fcntl64_nocancel.
* login/utmp_file.c: Likewise.
* sysdeps/posix/fdopendir.c: Likewise.
* sysdeps/posix/opendir.c: Likewise.
* sysdeps/unix/pt-fcntl.c: Likewise.
* include/fcntl.h (__libc_fcntl64, __fcntl64,
__fcntl64_nocancel_adjusted): New prototype.
(__fcntl_nocancel_adjusted): Remove prototype.
* io/Makefile (routines): Add fcntl64.
(CFLAGS-fcntl64.c): New rule.
* io/Versions [GLIBC_2.28] (fcntl64): New symbol.
[GLIBC_PRIVATE] (__libc_fcntl): Rename to __libc_fcntl64.
* io/fcntl.h (fcntl64): Add prototype and redirect if
__USE_FILE_OFFSET64 is defined.
* io/fcntl64.c: New file.
* manual/llio.text: Add a note for which commands fcntl acts a
cancellation point.
* nptl/Makefile (CFLAGS-fcntl64.c): New rule.
* sysdeps/mach/hurd/fcntl.c: Alias fcntl to fcntl64 symbols.
* sysdeps/mach/hurd/i386/libc.abilist [GLIBC_2.28] (fcntl, fcntl64):
New symbols.
* sysdeps/unix/sysv/linux/fcntl.c (__libc_fcntl): Fix F_GETLK64,
F_OFD_GETLK, F_SETLK64, F_SETLKW64, F_OFD_SETLK, and F_OFD_SETLKW for
non-LFS case.
* sysdeps/unix/sysv/linux/fcntl64.c: New file.
* sysdeps/unix/sysv/linux/fcntl_nocancel.c (__fcntl_nocancel): Rename
to __fcntl64_nocancel.
(__fcntl_nocancel_adjusted): Rename to __fcntl64_nocancel_adjusted.
* sysdeps/unix/sysv/linux/not-cancel.h (__fcntl_nocancel): Rename
to __fcntl64_nocancel.
* sysdeps/unix/sysv/linux/tst-ofdlocks.c: New file.
* sysdeps/unix/sysv/linux/tst-ofdlocks-compat.c: Likewise.
* sysdeps/unix/sysv/linux/Makefile (tests): Add tst-ofdlocks.
(tests-internal): Add tst-ofdlocks-compat.
* sysdeps/unix/sysv/linux/aarch64/libc.abilist [GLIBC_2.28]
(fcntl64): New symbol.
* sysdeps/unix/sysv/linux/alpha/libc.abilist: Likewise.
* sysdeps/unix/sysv/linux/ia64/libc.abilist: Likewise.
* sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist: Likewise.
* sysdeps/unix/sysv/linux/powerpc/powerpc64/libc.abilist: Likewise.
* sysdeps/unix/sysv/linux/powerpc/powerpc64/libc-le.abilist: Likewise.
* sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist: Likewise.
* sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist: Likewise.
* sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist: Likewise.
* sysdeps/unix/sysv/linux/x86_64/64/libc.abilist: Likewise.
* sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist: Likewise.
* sysdeps/unix/sysv/linux/arm/libc.abilist [GLIBC_2.28] (fcntl,
fcntl64): Likewise.
* sysdeps/unix/sysv/linux/hppa/libc.abilist: Likewise.
* sysdeps/unix/sysv/linux/i386/libc.abilis: Likewise.
* sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist: Likewise.
* sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist: Likewise.
* sysdeps/unix/sysv/linux/microblaze/libc.abilist: Likewise.
* sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist: Likewise.
* sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist: Likewise.
* sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist: Likewise.
* sysdeps/unix/sysv/linux/nios2/libc.abilist: Likewise.
* sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist:
Likewise.
* sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist:
Likewise.
* sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist: Likewise.
* sysdeps/unix/sysv/linux/sh/libc.abilist: Likewise.
* sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist: Likewise.
---
ChangeLog | 67 ++++++++++++++++
NEWS | 6 ++
csu/check_fds.c | 2 +-
include/fcntl.h | 7 +-
io/Makefile | 3 +-
io/Versions | 5 +-
io/fcntl.h | 11 +++
io/fcntl64.c | 38 +++++++++
login/utmp_file.c | 4 +-
manual/llio.texi | 13 ++--
nptl/Makefile | 1 +
sysdeps/mach/hurd/fcntl.c | 5 ++
sysdeps/mach/hurd/i386/libc.abilist | 2 +
sysdeps/posix/fdopendir.c | 2 +-
sysdeps/posix/opendir.c | 2 +-
sysdeps/unix/pt-fcntl.c | 2 +-
sysdeps/unix/sysv/linux/Makefile | 4 +-
sysdeps/unix/sysv/linux/aarch64/libc.abilist | 1 +
sysdeps/unix/sysv/linux/alpha/libc.abilist | 1 +
sysdeps/unix/sysv/linux/arm/libc.abilist | 2 +
sysdeps/unix/sysv/linux/fcntl.c | 89 +++++++++++++++++++---
sysdeps/unix/sysv/linux/fcntl64.c | 63 +++++++++++++++
sysdeps/unix/sysv/linux/fcntl_nocancel.c | 8 +-
sysdeps/unix/sysv/linux/hppa/libc.abilist | 2 +
sysdeps/unix/sysv/linux/i386/libc.abilist | 2 +
sysdeps/unix/sysv/linux/ia64/libc.abilist | 1 +
sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist | 2 +
sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist | 2 +
sysdeps/unix/sysv/linux/microblaze/libc.abilist | 2 +
.../unix/sysv/linux/mips/mips32/fpu/libc.abilist | 2 +
.../unix/sysv/linux/mips/mips32/nofpu/libc.abilist | 2 +
.../unix/sysv/linux/mips/mips64/n32/libc.abilist | 2 +
.../unix/sysv/linux/mips/mips64/n64/libc.abilist | 1 +
sysdeps/unix/sysv/linux/nios2/libc.abilist | 2 +
sysdeps/unix/sysv/linux/not-cancel.h | 4 +-
.../sysv/linux/powerpc/powerpc32/fpu/libc.abilist | 2 +
.../linux/powerpc/powerpc32/nofpu/libc.abilist | 2 +
.../sysv/linux/powerpc/powerpc64/libc-le.abilist | 1 +
.../unix/sysv/linux/powerpc/powerpc64/libc.abilist | 1 +
sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist | 1 +
sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist | 2 +
sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist | 1 +
sysdeps/unix/sysv/linux/sh/libc.abilist | 2 +
sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist | 2 +
sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist | 1 +
sysdeps/unix/sysv/linux/tst-ofdlocks-compat.c | 78 +++++++++++++++++++
sysdeps/unix/sysv/linux/tst-ofdlocks.c | 76 ++++++++++++++++++
sysdeps/unix/sysv/linux/x86_64/64/libc.abilist | 1 +
sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist | 1 +
49 files changed, 500 insertions(+), 33 deletions(-)
create mode 100644 io/fcntl64.c
create mode 100644 sysdeps/unix/sysv/linux/fcntl64.c
create mode 100644 sysdeps/unix/sysv/linux/tst-ofdlocks-compat.c
create mode 100644 sysdeps/unix/sysv/linux/tst-ofdlocks.c
Comments
On Wed, 20 Jun 2018, Adhemerval Zanella wrote:
> +#include <shlib-compat.h>
> +#if TEST_COMPAT (libc, GLIBC_2_0, GLIBC_2_28)
> +compat_symbol_reference (libc, fcntl, fcntl, GLIBC_2_0);
> +#endif
I'd expect the entire test to be under such a TEST_COMPAT conditional
(returning 77 for UNSUPPORTED otherwise) - for a new port without older
symbol versions, no such compat version with compat semantics is available
for testing.
On 20/06/2018 18:49, Joseph Myers wrote:
> On Wed, 20 Jun 2018, Adhemerval Zanella wrote:
>
>> +#include <shlib-compat.h>
>> +#if TEST_COMPAT (libc, GLIBC_2_0, GLIBC_2_28)
>> +compat_symbol_reference (libc, fcntl, fcntl, GLIBC_2_0);
>> +#endif
>
> I'd expect the entire test to be under such a TEST_COMPAT conditional
> (returning 77 for UNSUPPORTED otherwise) - for a new port without older
> symbol versions, no such compat version with compat semantics is available
> for testing.
>
Right, I forgot about news ports. I have adjusted it locally. Is this patch
ok to apply with this change?
On 06/20/2018 11:43 PM, Adhemerval Zanella wrote:
> diff --git a/sysdeps/unix/sysv/linux/fcntl.c b/sysdeps/unix/sysv/linux/fcntl.c
> index e3992dc..0e37ed0 100644
> --- a/sysdeps/unix/sysv/linux/fcntl.c
> +++ b/sysdeps/unix/sysv/linux/fcntl.c
> @@ -20,15 +20,12 @@
> #include <stdarg.h>
> #include <errno.h>
> #include <sysdep-cancel.h>
> -#include <not-cancel.h>
>
> -#ifndef __NR_fcntl64
> -# define __NR_fcntl64 __NR_fcntl
> -#endif
> +#ifndef __OFF_T_MATCHES_OFF64_T
>
> -#ifndef FCNTL_ADJUST_CMD
> -# define FCNTL_ADJUST_CMD(__cmd) __cmd
> -#endif
> +# ifndef FCNTL_ADJUST_CMD
> +# define FCNTL_ADJUST_CMD(__cmd) __cmd
> +# endif
>
> int
> __libc_fcntl (int fd, int cmd, ...)
> @@ -42,13 +39,83 @@ __libc_fcntl (int fd, int cmd, ...)
>
> cmd = FCNTL_ADJUST_CMD (cmd);
>
> - if (cmd == F_SETLKW || cmd == F_SETLKW64)
> - return SYSCALL_CANCEL (fcntl64, fd, cmd, (void *) arg);
> -
> - return __fcntl_nocancel_adjusted (fd, cmd, arg);
> + switch (cmd)
> + {
> + case F_SETLKW:
> + case F_SETLKW64:
> + return SYSCALL_CANCEL (fcntl64, fd, cmd, arg);
> + case F_OFD_SETLKW:
> + {
> + struct flock *flk = (struct flock *) arg;
> + struct flock64 flk64 =
> + {
> + .l_type = flk->l_type,
> + .l_whence = flk->l_whence,
> + .l_start = flk->l_start,
> + .l_len = flk->l_len,
> + .l_pid = flk->l_pid
> + };
> + return SYSCALL_CANCEL (fcntl64, fd, cmd, &flk64);
> + }
> + case F_OFD_GETLK:
> + case F_OFD_SETLK:
> + {
> + struct flock *flk = (struct flock *) arg;
> + struct flock64 flk64 =
> + {
> + .l_type = flk->l_type,
> + .l_whence = flk->l_whence,
> + .l_start = flk->l_start,
> + .l_len = flk->l_len,
> + .l_pid = flk->l_pid
> + };
> + int ret = INLINE_SYSCALL_CALL (fcntl64, fd, cmd, &flk64);
> + if (ret == -1)
> + return -1;
> + if ((off_t) flk64.l_start != flk64.l_start
> + || (off_t) flk64.l_len != flk64.l_len)
> + {
> + __set_errno (EOVERFLOW);
> + return -1;
> + }
> + flk->l_type = flk64.l_type;
> + flk->l_whence = flk64.l_whence;
> + flk->l_start = flk64.l_start;
> + flk->l_len = flk64.l_len;
> + flk->l_pid = flk64.l_pid;
> + return ret;
> + }
> + /* case F_OFD_GETLK:
> + case F_OFD_GETLK64:
> + case F_SETLK64:
> + case F_GETOWN: */
> + default:
> + return __fcntl64_nocancel_adjusted (fd, cmd, arg);
> + }
> }
The comment before the default case looks wrong to me. F_OFD_GETLK is
duplicated. Maybe add comments for the cases where mapping is not
needed, explaining why.
> libc_hidden_def (__libc_fcntl)
>
> weak_alias (__libc_fcntl, __fcntl)
> libc_hidden_weak (__fcntl)
> +
> +# include <shlib-compat.h>
> +# if SHLIB_COMPAT(libc, GLIBC_2_0, GLIBC_2_28)
> +int
> +__old_libc_fcntl64 (int fd, int cmd, ...)
> +{
> + va_list ap;
> + void *arg;
> +
> + va_start (ap, cmd);
> + arg = va_arg (ap, void *);
> + va_end (ap);
> +
> + return __libc_fcntl64 (fd, cmd, arg);
> +} > +compat_symbol (libc, __old_libc_fcntl64, fcntl, GLIBC_2_0);
This should have a comment why you call it __old_libc_fcntl64, when
there never was a fcntl64 before.
> +versioned_symbol (libc, __libc_fcntl, fcntl, GLIBC_2_28);
Doesn't this create a strong symbol, leading to static link namespace
issues?
> +# else
> weak_alias (__libc_fcntl, fcntl)
Here' it's a weak symbol.
Thanks,
Florian
On 22/06/2018 07:13, Florian Weimer wrote:
> On 06/20/2018 11:43 PM, Adhemerval Zanella wrote:
>> diff --git a/sysdeps/unix/sysv/linux/fcntl.c b/sysdeps/unix/sysv/linux/fcntl.c
>> index e3992dc..0e37ed0 100644
>> --- a/sysdeps/unix/sysv/linux/fcntl.c
>> +++ b/sysdeps/unix/sysv/linux/fcntl.c
>> @@ -20,15 +20,12 @@
>> #include <stdarg.h>
>> #include <errno.h>
>> #include <sysdep-cancel.h>
>> -#include <not-cancel.h>
>> -#ifndef __NR_fcntl64
>> -# define __NR_fcntl64 __NR_fcntl
>> -#endif
>> +#ifndef __OFF_T_MATCHES_OFF64_T
>> -#ifndef FCNTL_ADJUST_CMD
>> -# define FCNTL_ADJUST_CMD(__cmd) __cmd
>> -#endif
>> +# ifndef FCNTL_ADJUST_CMD
>> +# define FCNTL_ADJUST_CMD(__cmd) __cmd
>> +# endif
>> int
>> __libc_fcntl (int fd, int cmd, ...)
>> @@ -42,13 +39,83 @@ __libc_fcntl (int fd, int cmd, ...)
>> cmd = FCNTL_ADJUST_CMD (cmd);
>> - if (cmd == F_SETLKW || cmd == F_SETLKW64)
>> - return SYSCALL_CANCEL (fcntl64, fd, cmd, (void *) arg);
>> -
>> - return __fcntl_nocancel_adjusted (fd, cmd, arg);
>> + switch (cmd)
>> + {
>> + case F_SETLKW:
>> + case F_SETLKW64:
>> + return SYSCALL_CANCEL (fcntl64, fd, cmd, arg);
>> + case F_OFD_SETLKW:
>> + {
>> + struct flock *flk = (struct flock *) arg;
>> + struct flock64 flk64 =
>> + {
>> + .l_type = flk->l_type,
>> + .l_whence = flk->l_whence,
>> + .l_start = flk->l_start,
>> + .l_len = flk->l_len,
>> + .l_pid = flk->l_pid
>> + };
>> + return SYSCALL_CANCEL (fcntl64, fd, cmd, &flk64);
>> + }
>> + case F_OFD_GETLK:
>> + case F_OFD_SETLK:
>> + {
>> + struct flock *flk = (struct flock *) arg;
>> + struct flock64 flk64 =
>> + {
>> + .l_type = flk->l_type,
>> + .l_whence = flk->l_whence,
>> + .l_start = flk->l_start,
>> + .l_len = flk->l_len,
>> + .l_pid = flk->l_pid
>> + };
>> + int ret = INLINE_SYSCALL_CALL (fcntl64, fd, cmd, &flk64);
>> + if (ret == -1)
>> + return -1;
>> + if ((off_t) flk64.l_start != flk64.l_start
>> + || (off_t) flk64.l_len != flk64.l_len)
>> + {
>> + __set_errno (EOVERFLOW);
>> + return -1;
>> + }
>> + flk->l_type = flk64.l_type;
>> + flk->l_whence = flk64.l_whence;
>> + flk->l_start = flk64.l_start;
>> + flk->l_len = flk64.l_len;
>> + flk->l_pid = flk64.l_pid;
>> + return ret;
>> + }
>> + /* case F_OFD_GETLK:
>> + case F_OFD_GETLK64:
>> + case F_SETLK64:
>> + case F_GETOWN: */
>> + default:
>> + return __fcntl64_nocancel_adjusted (fd, cmd, arg);
>> + }
>> }
>
> The comment before the default case looks wrong to me. F_OFD_GETLK is duplicated. Maybe add comments for the cases where mapping is not needed, explaining why.
I changed to:
/* Since only F_SETLKW{64}/F_OLD_SETLK are cancellation entrypoints and
only OFD locks requires LFS handling, all others flags are handled
unmodified by calling __NR_fcntl64. */
>
>> libc_hidden_def (__libc_fcntl)
>> weak_alias (__libc_fcntl, __fcntl)
>> libc_hidden_weak (__fcntl)
>> +
>> +# include <shlib-compat.h>
>> +# if SHLIB_COMPAT(libc, GLIBC_2_0, GLIBC_2_28)
>> +int
>> +__old_libc_fcntl64 (int fd, int cmd, ...)
>> +{
>> + va_list ap;
>> + void *arg;
>> +
>> + va_start (ap, cmd);
>> + arg = va_arg (ap, void *);
>> + va_end (ap);
>> +
>> + return __libc_fcntl64 (fd, cmd, arg);
>> +} > +compat_symbol (libc, __old_libc_fcntl64, fcntl, GLIBC_2_0);
>
> This should have a comment why you call it __old_libc_fcntl64, when there never was a fcntl64 before.
I added.
/* Previous versions called __NR_fcntl64 for fcntl (which do not handle
OFD locks in LFS mode). */
>
>> +versioned_symbol (libc, __libc_fcntl, fcntl, GLIBC_2_28);
>
> Doesn't this create a strong symbol, leading to static link namespace issues?
The SHLIB_COMPAT takes care to avoid this in static objects.
>
>> +# else
>> weak_alias (__libc_fcntl, fcntl)
>
> Here' it's a weak symbol.
>
> Thanks,
> Florian
On Fri, 22 Jun 2018, Florian Weimer wrote:
> > +versioned_symbol (libc, __libc_fcntl, fcntl, GLIBC_2_28);
>
> Doesn't this create a strong symbol, leading to static link namespace issues?
For static linking, versioned_symbol is mapped to weak_alias.
On 06/22/2018 02:20 PM, Adhemerval Zanella wrote:
>>> + /* case F_OFD_GETLK:
>>> + case F_OFD_GETLK64:
>>> + case F_SETLK64:
>>> + case F_GETOWN: */
>>> + default:
>>> + return __fcntl64_nocancel_adjusted (fd, cmd, arg);
>>> + }
>>> }
>>
>> The comment before the default case looks wrong to me. F_OFD_GETLK is duplicated. Maybe add comments for the cases where mapping is not needed, explaining why.
>
> I changed to:
>
> /* Since only F_SETLKW{64}/F_OLD_SETLK are cancellation entrypoints and
> only OFD locks requires LFS handling, all others flags are handled
> unmodified by calling __NR_fcntl64. */
Thanks. I think it should read “only OFD locks require LFS handling”.
>>> +# include <shlib-compat.h>
>>> +# if SHLIB_COMPAT(libc, GLIBC_2_0, GLIBC_2_28)
>>> +int
>>> +__old_libc_fcntl64 (int fd, int cmd, ...)
>>> +{
>>> + va_list ap;
>>> + void *arg;
>>> +
>>> + va_start (ap, cmd);
>>> + arg = va_arg (ap, void *);
>>> + va_end (ap);
>>> +
>>> + return __libc_fcntl64 (fd, cmd, arg);
>>> +} > +compat_symbol (libc, __old_libc_fcntl64, fcntl, GLIBC_2_0);
>>
>> This should have a comment why you call it __old_libc_fcntl64, when there never was a fcntl64 before.
>
> I added.
>
> /* Previous versions called __NR_fcntl64 for fcntl (which do not handle
> OFD locks in LFS mode). */
“which did not handle”?
Florian
On 26/06/2018 11:45, Florian Weimer wrote:
> On 06/22/2018 02:20 PM, Adhemerval Zanella wrote:
>>>> + /* case F_OFD_GETLK:
>>>> + case F_OFD_GETLK64:
>>>> + case F_SETLK64:
>>>> + case F_GETOWN: */
>>>> + default:
>>>> + return __fcntl64_nocancel_adjusted (fd, cmd, arg);
>>>> + }
>>>> }
>>>
>>> The comment before the default case looks wrong to me. F_OFD_GETLK is duplicated. Maybe add comments for the cases where mapping is not needed, explaining why.
>>
>> I changed to:
>>
>> /* Since only F_SETLKW{64}/F_OLD_SETLK are cancellation entrypoints and
>> only OFD locks requires LFS handling, all others flags are handled
>> unmodified by calling __NR_fcntl64. */
>
> Thanks. I think it should read “only OFD locks require LFS handling”.
Fixed.
>
>>>> +# include <shlib-compat.h>
>>>> +# if SHLIB_COMPAT(libc, GLIBC_2_0, GLIBC_2_28)
>>>> +int
>>>> +__old_libc_fcntl64 (int fd, int cmd, ...)
>>>> +{
>>>> + va_list ap;
>>>> + void *arg;
>>>> +
>>>> + va_start (ap, cmd);
>>>> + arg = va_arg (ap, void *);
>>>> + va_end (ap);
>>>> +
>>>> + return __libc_fcntl64 (fd, cmd, arg);
>>>> +} > +compat_symbol (libc, __old_libc_fcntl64, fcntl, GLIBC_2_0);
>>>
>>> This should have a comment why you call it __old_libc_fcntl64, when there never was a fcntl64 before.
>>
>> I added.
>>
>> /* Previous versions called __NR_fcntl64 for fcntl (which do not handle
>> OFD locks in LFS mode). */
>
> “which did not handle”?
>
Indeed, fixed.
I will push it shortly.
On Tue, 26 Jun 2018, Adhemerval Zanella wrote:
> I will push it shortly.
The pushed commit includes a dubious change to
sysdeps/mach/hurd/bits/errno.h.
Maybe parts of that change are correct in that they are in fact the
results of regenerating that file, but if regenerating that file involves
inserting a line saying
"/home/azanella/Projects/glibc/build/i686-gnu/errnos.d", there is clearly
a bug in the generation process; generated checked-in files should never
depend on individual build paths like that.
On 26/06/2018 18:43, Joseph Myers wrote:
> On Tue, 26 Jun 2018, Adhemerval Zanella wrote:
>
>> I will push it shortly.
>
> The pushed commit includes a dubious change to
> sysdeps/mach/hurd/bits/errno.h.
>
> Maybe parts of that change are correct in that they are in fact the
> results of regenerating that file, but if regenerating that file involves
> inserting a line saying
> "/home/azanella/Projects/glibc/build/i686-gnu/errnos.d", there is clearly
> a bug in the generation process; generated checked-in files should never
> depend on individual build paths like that.
>
This is clearly a mistake from my part, I will revert this change. The
i686-gnu build seems to change this source file for some reason.
On Tue, 26 Jun 2018, Adhemerval Zanella wrote:
> On 26/06/2018 18:43, Joseph Myers wrote:
> > On Tue, 26 Jun 2018, Adhemerval Zanella wrote:
> >
> >> I will push it shortly.
> >
> > The pushed commit includes a dubious change to
> > sysdeps/mach/hurd/bits/errno.h.
> >
> > Maybe parts of that change are correct in that they are in fact the
> > results of regenerating that file, but if regenerating that file involves
> > inserting a line saying
> > "/home/azanella/Projects/glibc/build/i686-gnu/errnos.d", there is clearly
> > a bug in the generation process; generated checked-in files should never
> > depend on individual build paths like that.
> >
>
> This is clearly a mistake from my part, I will revert this change. The
> i686-gnu build seems to change this source file for some reason.
The files in the source tree that may have makefile dependencies on other
files in the source tree and so get regenerated during the build include
at least configure and configure fragments, some preconfigure fragments,
gperf-generated *-kw.h, INSTALL, sysdeps/gnu/errlist.c,
sysdeps/mach/hurd/bits/errno.h, posix/testcases.h, posix/ptestcases.h,
locale/C-translit.h, sysdeps/sparc/sparc32/{sdiv,udiv,rem,urem}.S.
(There may be others I've missed. Files such as po/libc.pot, with
makefile dependencies but which don't get referenced during a normal
build, aren't relevant for this purpose.)
Eventually it would be good for build-many-glibcs.py to touch all such
files in fix_glibc_timestamps to ensure that no regeneration in the source
tree takes place accidentally during a build (and given verification that
this makes a read-only source tree with any timestamp ordering work for
all supported configurations, build-many-glibcs.py could then stop copying
the glibc source tree at all). Of course that doesn't help so much when
not using build-many-glibcs.py; maybe we should have an equivalent of
GCC's contrib/gcc_update --touch that people could run during checkout
(and then build-many-glibcs.py would just use that).
On 06/20/2018 11:43 PM, Adhemerval Zanella wrote:
> Changes from previous version:
>
> - Add a testcase for compat fcntl using OFD locks.
>
> ---
>
> This patch fixes the OFD ("file private") locks for architectures that
> support non-LFS flock definition (__USE_FILE_OFFSET64 not defined). The
> issue in this case is both F_OFD_{GETLK,SETLK,SETLKW} and
> F_{SET,GET}L{W}K64 expects a flock64 argument and when using old
> F_OFD_* flags with a non LFS flock argument the kernel might interpret
> the underlying data wrongly. Kernel idea originally was to avoid using
> such flags in non-LFS syscall, but since GLIBC uses fcntl with LFS
> semantic as default it is possible to provide the functionality and
> avoid the bogus struct kernel passing by adjusting the struct manually
> for the required flags.
>
> The idea follows other LFS interfaces that provide two symbols:
>
> 1. A new LFS fcntl64 is added on default ABI with the usual macros to
> select it for FILE_OFFSET_BITS=64.
>
> 2. The Linux non-LFS fcntl use a stack allocated struct flock64 for
> F_OFD_{GETLK,SETLK,SETLKW} copy the results on the user provided
> struct.
>
> 3. Keep a compat symbol with old broken semantic for architectures
> that do not define __OFF_T_MATCHES_OFF64_T.
>
> So for architectures which defines __USE_FILE_OFFSET64, fcntl64 will
> aliased to fcntl and no adjustment would be required. So to actually
> use F_OFD_* with LFS support the source must be built with LFS support
> (_FILE_OFFSET_BITS=64).
>
> Also F_OFD_SETLKW command is handled a cancellation point, as for
> F_SETLKW{64}.
>
> Checked on x86_64-linux-gnu and i686-linux-gnu.
>
...
Hi Adhemerval,
I'm running the new test misc/tst-ofdlocks-compat on s390-32.
If I run it on linux 4.17, the test succeeds and after the second fcntl
call which returns zero, lck contains the region from the first fcntl call:
(gdb) p/x lck
$2 = {l_type = 0x1, l_whence = 0x0, l_start = 0x800003ff, l_len = 0x400,
l_pid = 0xffffffff}
If I run it on linux 4.14, the test fails. There the second fcntl
returns -1 and errno = EOVERFLOW
In this case, lck is not updated:
p/x lck
$4 = {l_type = 0x1, l_whence = 0x0, l_start = 0x7ffffbff, l_len =
0x1000, l_pid = 0x0}
In both cases struct flock64 is just passed to syscall fcntl64 via
__old_libc_fcntl64.
Are the different behaviours related to a change in kernel-code?
Bye
Stefan
On 06/07/2018 10:00, Stefan Liebler wrote:
> On 06/20/2018 11:43 PM, Adhemerval Zanella wrote:
>> Changes from previous version:
>>
>> - Add a testcase for compat fcntl using OFD locks.
>>
>> ---
>>
>> This patch fixes the OFD ("file private") locks for architectures that
>> support non-LFS flock definition (__USE_FILE_OFFSET64 not defined). The
>> issue in this case is both F_OFD_{GETLK,SETLK,SETLKW} and
>> F_{SET,GET}L{W}K64 expects a flock64 argument and when using old
>> F_OFD_* flags with a non LFS flock argument the kernel might interpret
>> the underlying data wrongly. Kernel idea originally was to avoid using
>> such flags in non-LFS syscall, but since GLIBC uses fcntl with LFS
>> semantic as default it is possible to provide the functionality and
>> avoid the bogus struct kernel passing by adjusting the struct manually
>> for the required flags.
>>
>> The idea follows other LFS interfaces that provide two symbols:
>>
>> 1. A new LFS fcntl64 is added on default ABI with the usual macros to
>> select it for FILE_OFFSET_BITS=64.
>>
>> 2. The Linux non-LFS fcntl use a stack allocated struct flock64 for
>> F_OFD_{GETLK,SETLK,SETLKW} copy the results on the user provided
>> struct.
>>
>> 3. Keep a compat symbol with old broken semantic for architectures
>> that do not define __OFF_T_MATCHES_OFF64_T.
>>
>> So for architectures which defines __USE_FILE_OFFSET64, fcntl64 will
>> aliased to fcntl and no adjustment would be required. So to actually
>> use F_OFD_* with LFS support the source must be built with LFS support
>> (_FILE_OFFSET_BITS=64).
>>
>> Also F_OFD_SETLKW command is handled a cancellation point, as for
>> F_SETLKW{64}.
>>
>> Checked on x86_64-linux-gnu and i686-linux-gnu.
>>
> ...
>
> Hi Adhemerval,
>
> I'm running the new test misc/tst-ofdlocks-compat on s390-32.
> If I run it on linux 4.17, the test succeeds and after the second fcntl call which returns zero, lck contains the region from the first fcntl call:
> (gdb) p/x lck
> $2 = {l_type = 0x1, l_whence = 0x0, l_start = 0x800003ff, l_len = 0x400, l_pid = 0xffffffff}
>
> If I run it on linux 4.14, the test fails. There the second fcntl returns -1 and errno = EOVERFLOW
> In this case, lck is not updated:
> p/x lck
> $4 = {l_type = 0x1, l_whence = 0x0, l_start = 0x7ffffbff, l_len = 0x1000, l_pid = 0x0}
>
> In both cases struct flock64 is just passed to syscall fcntl64 via __old_libc_fcntl64.
>
> Are the different behaviours related to a change in kernel-code?
I think it is due the patch 'fcntl: don't cap l_start and l_end values
for F_GETLK64 in compat syscall' (4d2dc2cc766c3b51929658cacbc6e34fc8e242fb
added on v4.15). Previously for COMPAT_SYSCALL_DEFINE3(fcntl64,...) kernel
did:
static long do_compat_fcntl64(unsigned int fd, unsigned int cmd,
compat_ulong_t arg)
[...]
case F_GETLK64:
case F_OFD_GETLK:
err = get_compat_flock64(&flock, compat_ptr(arg));
if (err)
break;
err = fixup_compat_flock(&flock);
if (err)
return err;
err = put_compat_flock64(&flock, compat_ptr(arg));
break;
[....]
It means that even if the fcntl(..., F_OFD_GETLK, struct flock64) did not
fail, kernel will return EOVERFLOW due 'fixup_compat_flock'. The patch
changed to:
[...]
case F_GETLK64:
case F_OFD_GETLK:
err = get_compat_flock64(&flock, compat_ptr(arg));
if (!err)
err= put_compat_flock64(&flock, compat_ptr(arg));
break;
[....]
So if compat fcntl will just return if get_compat_flock64 succeed. Also
afaiu only compat syscall is affected by it I think, the generic OFD
locks for 32 bits go through fcntl_getlk64 and it does return EOVERFLOW
in the aforementioned case.
I am not sure which would be the best way to get around this kernel
issue, any suggestion to harden the testcase? It also suggest to me that
the possible usercase I assume in my testcase (OFD locks with struct
flock64) never really worked on previous releases...
On 06/07/2018 11:09, Adhemerval Zanella wrote:
>
>
> On 06/07/2018 10:00, Stefan Liebler wrote:
>> On 06/20/2018 11:43 PM, Adhemerval Zanella wrote:
>>> Changes from previous version:
>>>
>>> - Add a testcase for compat fcntl using OFD locks.
>>>
>>> ---
>>>
>>> This patch fixes the OFD ("file private") locks for architectures that
>>> support non-LFS flock definition (__USE_FILE_OFFSET64 not defined). The
>>> issue in this case is both F_OFD_{GETLK,SETLK,SETLKW} and
>>> F_{SET,GET}L{W}K64 expects a flock64 argument and when using old
>>> F_OFD_* flags with a non LFS flock argument the kernel might interpret
>>> the underlying data wrongly. Kernel idea originally was to avoid using
>>> such flags in non-LFS syscall, but since GLIBC uses fcntl with LFS
>>> semantic as default it is possible to provide the functionality and
>>> avoid the bogus struct kernel passing by adjusting the struct manually
>>> for the required flags.
>>>
>>> The idea follows other LFS interfaces that provide two symbols:
>>>
>>> 1. A new LFS fcntl64 is added on default ABI with the usual macros to
>>> select it for FILE_OFFSET_BITS=64.
>>>
>>> 2. The Linux non-LFS fcntl use a stack allocated struct flock64 for
>>> F_OFD_{GETLK,SETLK,SETLKW} copy the results on the user provided
>>> struct.
>>>
>>> 3. Keep a compat symbol with old broken semantic for architectures
>>> that do not define __OFF_T_MATCHES_OFF64_T.
>>>
>>> So for architectures which defines __USE_FILE_OFFSET64, fcntl64 will
>>> aliased to fcntl and no adjustment would be required. So to actually
>>> use F_OFD_* with LFS support the source must be built with LFS support
>>> (_FILE_OFFSET_BITS=64).
>>>
>>> Also F_OFD_SETLKW command is handled a cancellation point, as for
>>> F_SETLKW{64}.
>>>
>>> Checked on x86_64-linux-gnu and i686-linux-gnu.
>>>
>> ...
>>
>> Hi Adhemerval,
>>
>> I'm running the new test misc/tst-ofdlocks-compat on s390-32.
>> If I run it on linux 4.17, the test succeeds and after the second fcntl call which returns zero, lck contains the region from the first fcntl call:
>> (gdb) p/x lck
>> $2 = {l_type = 0x1, l_whence = 0x0, l_start = 0x800003ff, l_len = 0x400, l_pid = 0xffffffff}
>>
>> If I run it on linux 4.14, the test fails. There the second fcntl returns -1 and errno = EOVERFLOW
>> In this case, lck is not updated:
>> p/x lck
>> $4 = {l_type = 0x1, l_whence = 0x0, l_start = 0x7ffffbff, l_len = 0x1000, l_pid = 0x0}
>>
>> In both cases struct flock64 is just passed to syscall fcntl64 via __old_libc_fcntl64.
>>
>> Are the different behaviours related to a change in kernel-code?
>
> I think it is due the patch 'fcntl: don't cap l_start and l_end values
> for F_GETLK64 in compat syscall' (4d2dc2cc766c3b51929658cacbc6e34fc8e242fb
> added on v4.15). Previously for COMPAT_SYSCALL_DEFINE3(fcntl64,...) kernel
> did:
>
> static long do_compat_fcntl64(unsigned int fd, unsigned int cmd,
> compat_ulong_t arg)
> [...]
> case F_GETLK64:
> case F_OFD_GETLK:
> err = get_compat_flock64(&flock, compat_ptr(arg));
> if (err)
> break;
> err = fixup_compat_flock(&flock);
> if (err)
> return err;
> err = put_compat_flock64(&flock, compat_ptr(arg));
> break;
> [....]
>
> It means that even if the fcntl(..., F_OFD_GETLK, struct flock64) did not
> fail, kernel will return EOVERFLOW due 'fixup_compat_flock'. The patch
> changed to:
>
> [...]
> case F_GETLK64:
> case F_OFD_GETLK:
> err = get_compat_flock64(&flock, compat_ptr(arg));
> if (!err)
> err= put_compat_flock64(&flock, compat_ptr(arg));
> break;
> [....]
>
> So if compat fcntl will just return if get_compat_flock64 succeed. Also
> afaiu only compat syscall is affected by it I think, the generic OFD
> locks for 32 bits go through fcntl_getlk64 and it does return EOVERFLOW
> in the aforementioned case.
>
> I am not sure which would be the best way to get around this kernel
> issue, any suggestion to harden the testcase? It also suggest to me that
> the possible usercase I assume in my testcase (OFD locks with struct
> flock64) never really worked on previous releases...
>
Also, on x86 4.4 where actually tested the kernel OFD locks does:
fs/compat.c:
[...]
case F_GETLK64:
case F_SETLK64:
case F_SETLKW64:
case F_OFD_GETLK:
case F_OFD_SETLK:
case F_OFD_SETLKW:
ret = get_compat_flock64(&f, compat_ptr(arg));
if (ret != 0)
break;
old_fs = get_fs();
set_fs(KERNEL_DS);
conv_cmd = convert_fcntl_cmd(cmd);
ret = sys_fcntl(fd, conv_cmd, (unsigned long)&f);
set_fs(old_fs);
if ((conv_cmd == F_GETLK || conv_cmd == F_OFD_GETLK) && ret == 0) {
/* need to return lock information - see above for commentary */
if (f.l_start > COMPAT_LOFF_T_MAX)
ret = -EOVERFLOW;
if (f.l_len > COMPAT_LOFF_T_MAX)
f.l_len = COMPAT_LOFF_T_MAX;
if (ret == 0)
ret = put_compat_flock64(&f, compat_ptr(arg));
}
break;
[...]
Which is similar to 4.14, however COMPAT_LOFF_T_MAX is defined as
0x7fffffffffffffffL for all architectures and l_start/l_len are
defined as __kernel_long_t (which for a 64 bits kernel is 'long').
So the tests are not actually checking for overflow.
Recent kernels do the right overflow check by using COMPAT_OFF_T_MAX
with a correct value (0x7fffffff on x86). It seems to be fixed by
80f0cce6aadebf6caf74d1f8ceb4b008ca72a9e9 (v4.12).
On 07/06/2018 04:45 PM, Adhemerval Zanella wrote:
>
>
> On 06/07/2018 11:09, Adhemerval Zanella wrote:
>>
>>
>> On 06/07/2018 10:00, Stefan Liebler wrote:
>>> On 06/20/2018 11:43 PM, Adhemerval Zanella wrote:
>>>> Changes from previous version:
>>>>
>>>> - Add a testcase for compat fcntl using OFD locks.
>>>>
>>>> ---
>>>>
>>>> This patch fixes the OFD ("file private") locks for architectures that
>>>> support non-LFS flock definition (__USE_FILE_OFFSET64 not defined). The
>>>> issue in this case is both F_OFD_{GETLK,SETLK,SETLKW} and
>>>> F_{SET,GET}L{W}K64 expects a flock64 argument and when using old
>>>> F_OFD_* flags with a non LFS flock argument the kernel might interpret
>>>> the underlying data wrongly. Kernel idea originally was to avoid using
>>>> such flags in non-LFS syscall, but since GLIBC uses fcntl with LFS
>>>> semantic as default it is possible to provide the functionality and
>>>> avoid the bogus struct kernel passing by adjusting the struct manually
>>>> for the required flags.
>>>>
>>>> The idea follows other LFS interfaces that provide two symbols:
>>>>
>>>> 1. A new LFS fcntl64 is added on default ABI with the usual macros to
>>>> select it for FILE_OFFSET_BITS=64.
>>>>
>>>> 2. The Linux non-LFS fcntl use a stack allocated struct flock64 for
>>>> F_OFD_{GETLK,SETLK,SETLKW} copy the results on the user provided
>>>> struct.
>>>>
>>>> 3. Keep a compat symbol with old broken semantic for architectures
>>>> that do not define __OFF_T_MATCHES_OFF64_T.
>>>>
>>>> So for architectures which defines __USE_FILE_OFFSET64, fcntl64 will
>>>> aliased to fcntl and no adjustment would be required. So to actually
>>>> use F_OFD_* with LFS support the source must be built with LFS support
>>>> (_FILE_OFFSET_BITS=64).
>>>>
>>>> Also F_OFD_SETLKW command is handled a cancellation point, as for
>>>> F_SETLKW{64}.
>>>>
>>>> Checked on x86_64-linux-gnu and i686-linux-gnu.
>>>>
>>> ...
>>>
>>> Hi Adhemerval,
>>>
>>> I'm running the new test misc/tst-ofdlocks-compat on s390-32.
>>> If I run it on linux 4.17, the test succeeds and after the second fcntl call which returns zero, lck contains the region from the first fcntl call:
>>> (gdb) p/x lck
>>> $2 = {l_type = 0x1, l_whence = 0x0, l_start = 0x800003ff, l_len = 0x400, l_pid = 0xffffffff}
>>>
>>> If I run it on linux 4.14, the test fails. There the second fcntl returns -1 and errno = EOVERFLOW
>>> In this case, lck is not updated:
>>> p/x lck
>>> $4 = {l_type = 0x1, l_whence = 0x0, l_start = 0x7ffffbff, l_len = 0x1000, l_pid = 0x0}
>>>
>>> In both cases struct flock64 is just passed to syscall fcntl64 via __old_libc_fcntl64.
>>>
>>> Are the different behaviours related to a change in kernel-code?
>>
>> I think it is due the patch 'fcntl: don't cap l_start and l_end values
>> for F_GETLK64 in compat syscall' (4d2dc2cc766c3b51929658cacbc6e34fc8e242fb
>> added on v4.15). Previously for COMPAT_SYSCALL_DEFINE3(fcntl64,...) kernel
>> did:
>>
>> static long do_compat_fcntl64(unsigned int fd, unsigned int cmd,
>> compat_ulong_t arg)
>> [...]
>> case F_GETLK64:
>> case F_OFD_GETLK:
>> err = get_compat_flock64(&flock, compat_ptr(arg));
>> if (err)
>> break;
>> err = fixup_compat_flock(&flock);
>> if (err)
>> return err;
>> err = put_compat_flock64(&flock, compat_ptr(arg));
>> break;
>> [....]
>>
>> It means that even if the fcntl(..., F_OFD_GETLK, struct flock64) did not
>> fail, kernel will return EOVERFLOW due 'fixup_compat_flock'. The patch
>> changed to:
>>
>> [...]
>> case F_GETLK64:
>> case F_OFD_GETLK:
>> err = get_compat_flock64(&flock, compat_ptr(arg));
>> if (!err)
>> err= put_compat_flock64(&flock, compat_ptr(arg));
>> break;
>> [....]
>>
Yes, this sounds reasonable.
It was introduced with commit 'fs/locks: don't mess with the address
limit in compat_fcntl64' (94073ad77fff221b5e66b8b9863a546ba212d6a3)
>> So if compat fcntl will just return if get_compat_flock64 succeed. Also
>> afaiu only compat syscall is affected by it I think, the generic OFD
>> locks for 32 bits go through fcntl_getlk64 and it does return EOVERFLOW
>> in the aforementioned case.
>>
>> I am not sure which would be the best way to get around this kernel
>> issue, any suggestion to harden the testcase? It also suggest to me that
>> the possible usercase I assume in my testcase (OFD locks with struct
>> flock64) never really worked on previous releases...
>>
>
> Also, on x86 4.4 where actually tested the kernel OFD locks does:
>
> fs/compat.c:
> [...]
> case F_GETLK64:
> case F_SETLK64:
> case F_SETLKW64:
> case F_OFD_GETLK:
> case F_OFD_SETLK:
> case F_OFD_SETLKW:
> ret = get_compat_flock64(&f, compat_ptr(arg));
> if (ret != 0)
> break;
> old_fs = get_fs();
> set_fs(KERNEL_DS);
> conv_cmd = convert_fcntl_cmd(cmd);
> ret = sys_fcntl(fd, conv_cmd, (unsigned long)&f);
> set_fs(old_fs);
> if ((conv_cmd == F_GETLK || conv_cmd == F_OFD_GETLK) && ret == 0) {
> /* need to return lock information - see above for commentary */
> if (f.l_start > COMPAT_LOFF_T_MAX)
> ret = -EOVERFLOW;
> if (f.l_len > COMPAT_LOFF_T_MAX)
> f.l_len = COMPAT_LOFF_T_MAX;
> if (ret == 0)
> ret = put_compat_flock64(&f, compat_ptr(arg));
> }
> break;
> [...]
>
> Which is similar to 4.14, however COMPAT_LOFF_T_MAX is defined as
> 0x7fffffffffffffffL for all architectures and l_start/l_len are
> defined as __kernel_long_t (which for a 64 bits kernel is 'long').
> So the tests are not actually checking for overflow.
Yes. They do not test for overflowing 32bit long. But as fcntl64 with
command F_OFD_GETLK assumes 64bit long, it is fine?
>
> Recent kernels do the right overflow check by using COMPAT_OFF_T_MAX
> with a correct value (0x7fffffff on x86). It seems to be fixed by
> 80f0cce6aadebf6caf74d1f8ceb4b008ca72a9e9 (v4.12).
>
Sure? This commit introduces the following implementation and I think
those checks are the same as before:
COMPAT_SYSCALL_DEFINE3(fcntl64,...
...
case F_GETLK:
... if (f.l_start > COMPAT_OFF_T_MAX)
ret = -EOVERFLOW;
...
case F_OFD_GETLK:
... if (f.l_start > COMPAT_LOFF_T_MAX)
ret = -EOVERFLOW;
In recent kernels, only the command F_GETLK is using
fixup_compat_flock() which checks against COMPAT_OFF_T_MAX.
The errno=EOVERFLOW for the second fcntl call in tst-ofdlocks.c is
generated by the new glibc check, but not from the kernel.
I've got errno=EOVERFLOW with my kernel 4.14 due to the call of
fixup_compat_flock.
I've also retested it on a system with kernel 4.10. There the kernel
does not return with errno=EOVERFLOW.
On 09/07/2018 10:44, Stefan Liebler wrote:
> On 07/06/2018 04:45 PM, Adhemerval Zanella wrote:
>>
>>
>> On 06/07/2018 11:09, Adhemerval Zanella wrote:
>>>
>>>
>>> On 06/07/2018 10:00, Stefan Liebler wrote:
>>>> On 06/20/2018 11:43 PM, Adhemerval Zanella wrote:
>>>>> Changes from previous version:
>>>>>
>>>>> - Add a testcase for compat fcntl using OFD locks.
>>>>>
>>>>> ---
>>>>>
>>>>> This patch fixes the OFD ("file private") locks for architectures that
>>>>> support non-LFS flock definition (__USE_FILE_OFFSET64 not defined). The
>>>>> issue in this case is both F_OFD_{GETLK,SETLK,SETLKW} and
>>>>> F_{SET,GET}L{W}K64 expects a flock64 argument and when using old
>>>>> F_OFD_* flags with a non LFS flock argument the kernel might interpret
>>>>> the underlying data wrongly. Kernel idea originally was to avoid using
>>>>> such flags in non-LFS syscall, but since GLIBC uses fcntl with LFS
>>>>> semantic as default it is possible to provide the functionality and
>>>>> avoid the bogus struct kernel passing by adjusting the struct manually
>>>>> for the required flags.
>>>>>
>>>>> The idea follows other LFS interfaces that provide two symbols:
>>>>>
>>>>> 1. A new LFS fcntl64 is added on default ABI with the usual macros to
>>>>> select it for FILE_OFFSET_BITS=64.
>>>>>
>>>>> 2. The Linux non-LFS fcntl use a stack allocated struct flock64 for
>>>>> F_OFD_{GETLK,SETLK,SETLKW} copy the results on the user provided
>>>>> struct.
>>>>>
>>>>> 3. Keep a compat symbol with old broken semantic for architectures
>>>>> that do not define __OFF_T_MATCHES_OFF64_T.
>>>>>
>>>>> So for architectures which defines __USE_FILE_OFFSET64, fcntl64 will
>>>>> aliased to fcntl and no adjustment would be required. So to actually
>>>>> use F_OFD_* with LFS support the source must be built with LFS support
>>>>> (_FILE_OFFSET_BITS=64).
>>>>>
>>>>> Also F_OFD_SETLKW command is handled a cancellation point, as for
>>>>> F_SETLKW{64}.
>>>>>
>>>>> Checked on x86_64-linux-gnu and i686-linux-gnu.
>>>>>
>>>> ...
>>>>
>>>> Hi Adhemerval,
>>>>
>>>> I'm running the new test misc/tst-ofdlocks-compat on s390-32.
>>>> If I run it on linux 4.17, the test succeeds and after the second fcntl call which returns zero, lck contains the region from the first fcntl call:
>>>> (gdb) p/x lck
>>>> $2 = {l_type = 0x1, l_whence = 0x0, l_start = 0x800003ff, l_len = 0x400, l_pid = 0xffffffff}
>>>>
>>>> If I run it on linux 4.14, the test fails. There the second fcntl returns -1 and errno = EOVERFLOW
>>>> In this case, lck is not updated:
>>>> p/x lck
>>>> $4 = {l_type = 0x1, l_whence = 0x0, l_start = 0x7ffffbff, l_len = 0x1000, l_pid = 0x0}
>>>>
>>>> In both cases struct flock64 is just passed to syscall fcntl64 via __old_libc_fcntl64.
>>>>
>>>> Are the different behaviours related to a change in kernel-code?
>>>
>>> I think it is due the patch 'fcntl: don't cap l_start and l_end values
>>> for F_GETLK64 in compat syscall' (4d2dc2cc766c3b51929658cacbc6e34fc8e242fb
>>> added on v4.15). Previously for COMPAT_SYSCALL_DEFINE3(fcntl64,...) kernel
>>> did:
>>>
>>> static long do_compat_fcntl64(unsigned int fd, unsigned int cmd,
>>> compat_ulong_t arg)
>>> [...]
>>> case F_GETLK64:
>>> case F_OFD_GETLK:
>>> err = get_compat_flock64(&flock, compat_ptr(arg));
>>> if (err)
>>> break;
>>> err = fixup_compat_flock(&flock);
>>> if (err)
>>> return err;
>>> err = put_compat_flock64(&flock, compat_ptr(arg));
>>> break;
>>> [....]
>>>
>>> It means that even if the fcntl(..., F_OFD_GETLK, struct flock64) did not
>>> fail, kernel will return EOVERFLOW due 'fixup_compat_flock'. The patch
>>> changed to:
>>>
>>> [...]
>>> case F_GETLK64:
>>> case F_OFD_GETLK:
>>> err = get_compat_flock64(&flock, compat_ptr(arg));
>>> if (!err)
>>> err= put_compat_flock64(&flock, compat_ptr(arg));
>>> break;
>>> [....]
>>>
> Yes, this sounds reasonable.
> It was introduced with commit 'fs/locks: don't mess with the address limit in compat_fcntl64' (94073ad77fff221b5e66b8b9863a546ba212d6a3)
>
>>> So if compat fcntl will just return if get_compat_flock64 succeed. Also
>>> afaiu only compat syscall is affected by it I think, the generic OFD
>>> locks for 32 bits go through fcntl_getlk64 and it does return EOVERFLOW
>>> in the aforementioned case.
>>>
>>> I am not sure which would be the best way to get around this kernel
>>> issue, any suggestion to harden the testcase? It also suggest to me that
>>> the possible usercase I assume in my testcase (OFD locks with struct
>>> flock64) never really worked on previous releases...
>>>
>>
>> Also, on x86 4.4 where actually tested the kernel OFD locks does:
>>
>> fs/compat.c:
>> [...]
>> case F_GETLK64:
>> case F_SETLK64:
>> case F_SETLKW64:
>> case F_OFD_GETLK:
>> case F_OFD_SETLK:
>> case F_OFD_SETLKW:
>> ret = get_compat_flock64(&f, compat_ptr(arg));
>> if (ret != 0)
>> break;
>> old_fs = get_fs();
>> set_fs(KERNEL_DS);
>> conv_cmd = convert_fcntl_cmd(cmd);
>> ret = sys_fcntl(fd, conv_cmd, (unsigned long)&f);
>> set_fs(old_fs);
>> if ((conv_cmd == F_GETLK || conv_cmd == F_OFD_GETLK) && ret == 0) {
>> /* need to return lock information - see above for commentary */
>> if (f.l_start > COMPAT_LOFF_T_MAX)
>> ret = -EOVERFLOW;
>> if (f.l_len > COMPAT_LOFF_T_MAX)
>> f.l_len = COMPAT_LOFF_T_MAX;
>> if (ret == 0)
>> ret = put_compat_flock64(&f, compat_ptr(arg));
>> }
>> break;
>> [...]
>>
>> Which is similar to 4.14, however COMPAT_LOFF_T_MAX is defined as
>> 0x7fffffffffffffffL for all architectures and l_start/l_len are
>> defined as __kernel_long_t (which for a 64 bits kernel is 'long').
>> So the tests are not actually checking for overflow.
> Yes. They do not test for overflowing 32bit long. But as fcntl64 with command F_OFD_GETLK assumes 64bit long, it is fine?
>>
>> Recent kernels do the right overflow check by using COMPAT_OFF_T_MAX
>> with a correct value (0x7fffffff on x86). It seems to be fixed by
>> 80f0cce6aadebf6caf74d1f8ceb4b008ca72a9e9 (v4.12).
>>
> Sure? This commit introduces the following implementation and I think those checks are the same as before:
> COMPAT_SYSCALL_DEFINE3(fcntl64,...
Not sure, I just try to see if there a kernel change by inspecting the
kernel patches.
> ...
> case F_GETLK:
> ... if (f.l_start > COMPAT_OFF_T_MAX)
> ret = -EOVERFLOW;
> ...
> case F_OFD_GETLK:
> ... if (f.l_start > COMPAT_LOFF_T_MAX)
> ret = -EOVERFLOW;
>
> In recent kernels, only the command F_GETLK is using fixup_compat_flock() which checks against COMPAT_OFF_T_MAX.
>
> The errno=EOVERFLOW for the second fcntl call in tst-ofdlocks.c is generated by the new glibc check, but not from the kernel.
This is the expected behaviour, the new fcntl symbol for non default LFS
ABI returns EOVERFLOW.
> I've got errno=EOVERFLOW with my kernel 4.14 due to the call of fixup_compat_flock.
> I've also retested it on a system with kernel 4.10. There the kernel does not return with errno=EOVERFLOW.
>
Can you verify that for s390-32 tst-ofdlocks-compat.c is a really using the
old version (i.e passing the arguments to kernel unmodified)? Otherwise
I am not sure if this is a glibc bug.
On 07/09/2018 04:20 PM, Adhemerval Zanella wrote:
>
>
> On 09/07/2018 10:44, Stefan Liebler wrote:
>> On 07/06/2018 04:45 PM, Adhemerval Zanella wrote:
>>>
>>>
>>> On 06/07/2018 11:09, Adhemerval Zanella wrote:
>>>>
>>>>
>>>> On 06/07/2018 10:00, Stefan Liebler wrote:
>>>>> On 06/20/2018 11:43 PM, Adhemerval Zanella wrote:
>>>>>> Changes from previous version:
>>>>>>
>>>>>> - Add a testcase for compat fcntl using OFD locks.
>>>>>>
>>>>>> ---
>>>>>>
>>>>>> This patch fixes the OFD ("file private") locks for architectures that
>>>>>> support non-LFS flock definition (__USE_FILE_OFFSET64 not defined). The
>>>>>> issue in this case is both F_OFD_{GETLK,SETLK,SETLKW} and
>>>>>> F_{SET,GET}L{W}K64 expects a flock64 argument and when using old
>>>>>> F_OFD_* flags with a non LFS flock argument the kernel might interpret
>>>>>> the underlying data wrongly. Kernel idea originally was to avoid using
>>>>>> such flags in non-LFS syscall, but since GLIBC uses fcntl with LFS
>>>>>> semantic as default it is possible to provide the functionality and
>>>>>> avoid the bogus struct kernel passing by adjusting the struct manually
>>>>>> for the required flags.
>>>>>>
>>>>>> The idea follows other LFS interfaces that provide two symbols:
>>>>>>
>>>>>> 1. A new LFS fcntl64 is added on default ABI with the usual macros to
>>>>>> select it for FILE_OFFSET_BITS=64.
>>>>>>
>>>>>> 2. The Linux non-LFS fcntl use a stack allocated struct flock64 for
>>>>>> F_OFD_{GETLK,SETLK,SETLKW} copy the results on the user provided
>>>>>> struct.
>>>>>>
>>>>>> 3. Keep a compat symbol with old broken semantic for architectures
>>>>>> that do not define __OFF_T_MATCHES_OFF64_T.
>>>>>>
>>>>>> So for architectures which defines __USE_FILE_OFFSET64, fcntl64 will
>>>>>> aliased to fcntl and no adjustment would be required. So to actually
>>>>>> use F_OFD_* with LFS support the source must be built with LFS support
>>>>>> (_FILE_OFFSET_BITS=64).
>>>>>>
>>>>>> Also F_OFD_SETLKW command is handled a cancellation point, as for
>>>>>> F_SETLKW{64}.
>>>>>>
>>>>>> Checked on x86_64-linux-gnu and i686-linux-gnu.
>>>>>>
>>>>> ...
>>>>>
>>>>> Hi Adhemerval,
>>>>>
>>>>> I'm running the new test misc/tst-ofdlocks-compat on s390-32.
>>>>> If I run it on linux 4.17, the test succeeds and after the second fcntl call which returns zero, lck contains the region from the first fcntl call:
>>>>> (gdb) p/x lck
>>>>> $2 = {l_type = 0x1, l_whence = 0x0, l_start = 0x800003ff, l_len = 0x400, l_pid = 0xffffffff}
>>>>>
>>>>> If I run it on linux 4.14, the test fails. There the second fcntl returns -1 and errno = EOVERFLOW
>>>>> In this case, lck is not updated:
>>>>> p/x lck
>>>>> $4 = {l_type = 0x1, l_whence = 0x0, l_start = 0x7ffffbff, l_len = 0x1000, l_pid = 0x0}
>>>>>
>>>>> In both cases struct flock64 is just passed to syscall fcntl64 via __old_libc_fcntl64.
>>>>>
>>>>> Are the different behaviours related to a change in kernel-code?
>>>>
>>>> I think it is due the patch 'fcntl: don't cap l_start and l_end values
>>>> for F_GETLK64 in compat syscall' (4d2dc2cc766c3b51929658cacbc6e34fc8e242fb
>>>> added on v4.15). Previously for COMPAT_SYSCALL_DEFINE3(fcntl64,...) kernel
>>>> did:
>>>>
>>>> static long do_compat_fcntl64(unsigned int fd, unsigned int cmd,
>>>> compat_ulong_t arg)
>>>> [...]
>>>> case F_GETLK64:
>>>> case F_OFD_GETLK:
>>>> err = get_compat_flock64(&flock, compat_ptr(arg));
>>>> if (err)
>>>> break;
>>>> err = fixup_compat_flock(&flock);
>>>> if (err)
>>>> return err;
>>>> err = put_compat_flock64(&flock, compat_ptr(arg));
>>>> break;
>>>> [....]
>>>>
>>>> It means that even if the fcntl(..., F_OFD_GETLK, struct flock64) did not
>>>> fail, kernel will return EOVERFLOW due 'fixup_compat_flock'. The patch
>>>> changed to:
>>>>
>>>> [...]
>>>> case F_GETLK64:
>>>> case F_OFD_GETLK:
>>>> err = get_compat_flock64(&flock, compat_ptr(arg));
>>>> if (!err)
>>>> err= put_compat_flock64(&flock, compat_ptr(arg));
>>>> break;
>>>> [....]
>>>>
>> Yes, this sounds reasonable.
>> It was introduced with commit 'fs/locks: don't mess with the address limit in compat_fcntl64' (94073ad77fff221b5e66b8b9863a546ba212d6a3)
>>
>>>> So if compat fcntl will just return if get_compat_flock64 succeed. Also
>>>> afaiu only compat syscall is affected by it I think, the generic OFD
>>>> locks for 32 bits go through fcntl_getlk64 and it does return EOVERFLOW
>>>> in the aforementioned case.
>>>>
>>>> I am not sure which would be the best way to get around this kernel
>>>> issue, any suggestion to harden the testcase? It also suggest to me that
>>>> the possible usercase I assume in my testcase (OFD locks with struct
>>>> flock64) never really worked on previous releases...
>>>>
>>>
>>> Also, on x86 4.4 where actually tested the kernel OFD locks does:
>>>
>>> fs/compat.c:
>>> [...]
>>> case F_GETLK64:
>>> case F_SETLK64:
>>> case F_SETLKW64:
>>> case F_OFD_GETLK:
>>> case F_OFD_SETLK:
>>> case F_OFD_SETLKW:
>>> ret = get_compat_flock64(&f, compat_ptr(arg));
>>> if (ret != 0)
>>> break;
>>> old_fs = get_fs();
>>> set_fs(KERNEL_DS);
>>> conv_cmd = convert_fcntl_cmd(cmd);
>>> ret = sys_fcntl(fd, conv_cmd, (unsigned long)&f);
>>> set_fs(old_fs);
>>> if ((conv_cmd == F_GETLK || conv_cmd == F_OFD_GETLK) && ret == 0) {
>>> /* need to return lock information - see above for commentary */
>>> if (f.l_start > COMPAT_LOFF_T_MAX)
>>> ret = -EOVERFLOW;
>>> if (f.l_len > COMPAT_LOFF_T_MAX)
>>> f.l_len = COMPAT_LOFF_T_MAX;
>>> if (ret == 0)
>>> ret = put_compat_flock64(&f, compat_ptr(arg));
>>> }
>>> break;
>>> [...]
>>>
>>> Which is similar to 4.14, however COMPAT_LOFF_T_MAX is defined as
>>> 0x7fffffffffffffffL for all architectures and l_start/l_len are
>>> defined as __kernel_long_t (which for a 64 bits kernel is 'long').
>>> So the tests are not actually checking for overflow.
>> Yes. They do not test for overflowing 32bit long. But as fcntl64 with command F_OFD_GETLK assumes 64bit long, it is fine?
>>>
>>> Recent kernels do the right overflow check by using COMPAT_OFF_T_MAX
>>> with a correct value (0x7fffffff on x86). It seems to be fixed by
>>> 80f0cce6aadebf6caf74d1f8ceb4b008ca72a9e9 (v4.12).
>>>
>> Sure? This commit introduces the following implementation and I think those checks are the same as before:
>> COMPAT_SYSCALL_DEFINE3(fcntl64,...
>
> Not sure, I just try to see if there a kernel change by inspecting the
> kernel patches.
>
>> ...
>> case F_GETLK:
>> ... if (f.l_start > COMPAT_OFF_T_MAX)
>> ret = -EOVERFLOW;
>> ...
>> case F_OFD_GETLK:
>> ... if (f.l_start > COMPAT_LOFF_T_MAX)
>> ret = -EOVERFLOW;
>>
>> In recent kernels, only the command F_GETLK is using fixup_compat_flock() which checks against COMPAT_OFF_T_MAX.
>>
>> The errno=EOVERFLOW for the second fcntl call in tst-ofdlocks.c is generated by the new glibc check, but not from the kernel.
>
> This is the expected behaviour, the new fcntl symbol for non default LFS
> ABI returns EOVERFLOW.
>
>> I've got errno=EOVERFLOW with my kernel 4.14 due to the call of fixup_compat_flock.
>> I've also retested it on a system with kernel 4.10. There the kernel does not return with errno=EOVERFLOW.
>>
>
> Can you verify that for s390-32 tst-ofdlocks-compat.c is a really using the
> old version (i.e passing the arguments to kernel unmodified)? Otherwise
> I am not sure if this is a glibc bug.
>
Yes it is. I'm running the s390-32 tst-ofdlocks-compat testcase in 64bit
gdb and we are here:
72 TEST_VERIFY (fcntl (fd, F_OFD_GETLK, &lck) == 0);
(gdb) p sizeof (lck)
$2 = 32
(gdb) p/x lck
$3 = {l_type = 0x1, l_whence = 0x0, l_start = 0x7ffffbff, l_len = 0x1000,
l_pid = 0x0}
(gdb) x/4gx &lck
0x7fffeae8: 0x0001000000000000 0x000000007ffffbff
0x7fffeaf8: 0x0000000000001000 0x0000000000000000
Stepping to syscall: ...
(gdb) where
#0 __fcntl64_nocancel_adjusted (fd=fd@entry=4, cmd=cmd@entry=36,
arg=arg@entry=0x7fffeae8)
at ../sysdeps/unix/sysv/linux/fcntl_nocancel.c:64
#1 0x7df1e076 in __GI___libc_fcntl64 (fd=4, cmd=36) at
../sysdeps/unix/sysv/linux/fcntl64.c:51
#2 0x7df1e006 in __old_libc_fcntl64 (fd=<optimized out>, cmd=<optimized
out>) at ../sysdeps/unix/sysv/linux/fcntl.c:114
#3 0x00401a98 in do_test () at
../sysdeps/unix/sysv/linux/tst-ofdlocks-compat.c:72
#4 0x00401f42 in run_test_function (config=0x7fffebe8,
config=0x7fffebe8, argv=0x7fffed48, argc=<optimized out>)
at support_test_main.c:156
#5 support_test_main (argc=<optimized out>, argv=0x7fffed48,
config=config@entry=0x7fffebe8) at support_test_main.c:322
#6 0x004017b0 in main (argc=<optimized out>, argv=<optimized out>) at
../support/test-driver.c:168
We are just before the syscall:
>│0x7df25fe0 <__fcntl64_nocancel_adjusted+32> svc 221
(gdb) x/4gx arg
0x7fffeae8: 0x0001000000000000 0x000000007ffffbff
0x7fffeaf8: 0x0000000000001000 0x0000000000000000
(gdb) si
(gdb) ir 2
r2 0xffffffffffffffb5 18446744073709551541
(gdb) x/4gx arg
0x7fffeae8: 0x0001000000000000 0x000000007ffffbff
0x7fffeaf8: 0x0000000000001000 0x0000000000000000
On 09/07/2018 11:56, Stefan Liebler wrote:
> On 07/09/2018 04:20 PM, Adhemerval Zanella wrote:
>>
>>
>> On 09/07/2018 10:44, Stefan Liebler wrote:
>>> On 07/06/2018 04:45 PM, Adhemerval Zanella wrote:
>>>>
>>>>
>>>> On 06/07/2018 11:09, Adhemerval Zanella wrote:
>>>>>
>>>>>
>>>>> On 06/07/2018 10:00, Stefan Liebler wrote:
>>>>>> On 06/20/2018 11:43 PM, Adhemerval Zanella wrote:
>>>>>>> Changes from previous version:
>>>>>>>
>>>>>>> - Add a testcase for compat fcntl using OFD locks.
>>>>>>>
>>>>>>> ---
>>>>>>>
>>>>>>> This patch fixes the OFD ("file private") locks for architectures that
>>>>>>> support non-LFS flock definition (__USE_FILE_OFFSET64 not defined). The
>>>>>>> issue in this case is both F_OFD_{GETLK,SETLK,SETLKW} and
>>>>>>> F_{SET,GET}L{W}K64 expects a flock64 argument and when using old
>>>>>>> F_OFD_* flags with a non LFS flock argument the kernel might interpret
>>>>>>> the underlying data wrongly. Kernel idea originally was to avoid using
>>>>>>> such flags in non-LFS syscall, but since GLIBC uses fcntl with LFS
>>>>>>> semantic as default it is possible to provide the functionality and
>>>>>>> avoid the bogus struct kernel passing by adjusting the struct manually
>>>>>>> for the required flags.
>>>>>>>
>>>>>>> The idea follows other LFS interfaces that provide two symbols:
>>>>>>>
>>>>>>> 1. A new LFS fcntl64 is added on default ABI with the usual macros to
>>>>>>> select it for FILE_OFFSET_BITS=64.
>>>>>>>
>>>>>>> 2. The Linux non-LFS fcntl use a stack allocated struct flock64 for
>>>>>>> F_OFD_{GETLK,SETLK,SETLKW} copy the results on the user provided
>>>>>>> struct.
>>>>>>>
>>>>>>> 3. Keep a compat symbol with old broken semantic for architectures
>>>>>>> that do not define __OFF_T_MATCHES_OFF64_T.
>>>>>>>
>>>>>>> So for architectures which defines __USE_FILE_OFFSET64, fcntl64 will
>>>>>>> aliased to fcntl and no adjustment would be required. So to actually
>>>>>>> use F_OFD_* with LFS support the source must be built with LFS support
>>>>>>> (_FILE_OFFSET_BITS=64).
>>>>>>>
>>>>>>> Also F_OFD_SETLKW command is handled a cancellation point, as for
>>>>>>> F_SETLKW{64}.
>>>>>>>
>>>>>>> Checked on x86_64-linux-gnu and i686-linux-gnu.
>>>>>>>
>>>>>> ...
>>>>>>
>>>>>> Hi Adhemerval,
>>>>>>
>>>>>> I'm running the new test misc/tst-ofdlocks-compat on s390-32.
>>>>>> If I run it on linux 4.17, the test succeeds and after the second fcntl call which returns zero, lck contains the region from the first fcntl call:
>>>>>> (gdb) p/x lck
>>>>>> $2 = {l_type = 0x1, l_whence = 0x0, l_start = 0x800003ff, l_len = 0x400, l_pid = 0xffffffff}
>>>>>>
>>>>>> If I run it on linux 4.14, the test fails. There the second fcntl returns -1 and errno = EOVERFLOW
>>>>>> In this case, lck is not updated:
>>>>>> p/x lck
>>>>>> $4 = {l_type = 0x1, l_whence = 0x0, l_start = 0x7ffffbff, l_len = 0x1000, l_pid = 0x0}
>>>>>>
>>>>>> In both cases struct flock64 is just passed to syscall fcntl64 via __old_libc_fcntl64.
>>>>>>
>>>>>> Are the different behaviours related to a change in kernel-code?
>>>>>
>>>>> I think it is due the patch 'fcntl: don't cap l_start and l_end values
>>>>> for F_GETLK64 in compat syscall' (4d2dc2cc766c3b51929658cacbc6e34fc8e242fb
>>>>> added on v4.15). Previously for COMPAT_SYSCALL_DEFINE3(fcntl64,...) kernel
>>>>> did:
>>>>>
>>>>> static long do_compat_fcntl64(unsigned int fd, unsigned int cmd,
>>>>> compat_ulong_t arg)
>>>>> [...]
>>>>> case F_GETLK64:
>>>>> case F_OFD_GETLK:
>>>>> err = get_compat_flock64(&flock, compat_ptr(arg));
>>>>> if (err)
>>>>> break;
>>>>> err = fixup_compat_flock(&flock);
>>>>> if (err)
>>>>> return err;
>>>>> err = put_compat_flock64(&flock, compat_ptr(arg));
>>>>> break;
>>>>> [....]
>>>>>
>>>>> It means that even if the fcntl(..., F_OFD_GETLK, struct flock64) did not
>>>>> fail, kernel will return EOVERFLOW due 'fixup_compat_flock'. The patch
>>>>> changed to:
>>>>>
>>>>> [...]
>>>>> case F_GETLK64:
>>>>> case F_OFD_GETLK:
>>>>> err = get_compat_flock64(&flock, compat_ptr(arg));
>>>>> if (!err)
>>>>> err= put_compat_flock64(&flock, compat_ptr(arg));
>>>>> break;
>>>>> [....]
>>>>>
>>> Yes, this sounds reasonable.
>>> It was introduced with commit 'fs/locks: don't mess with the address limit in compat_fcntl64' (94073ad77fff221b5e66b8b9863a546ba212d6a3)
>>>
>>>>> So if compat fcntl will just return if get_compat_flock64 succeed. Also
>>>>> afaiu only compat syscall is affected by it I think, the generic OFD
>>>>> locks for 32 bits go through fcntl_getlk64 and it does return EOVERFLOW
>>>>> in the aforementioned case.
>>>>>
>>>>> I am not sure which would be the best way to get around this kernel
>>>>> issue, any suggestion to harden the testcase? It also suggest to me that
>>>>> the possible usercase I assume in my testcase (OFD locks with struct
>>>>> flock64) never really worked on previous releases...
>>>>>
>>>>
>>>> Also, on x86 4.4 where actually tested the kernel OFD locks does:
>>>>
>>>> fs/compat.c:
>>>> [...]
>>>> case F_GETLK64:
>>>> case F_SETLK64:
>>>> case F_SETLKW64:
>>>> case F_OFD_GETLK:
>>>> case F_OFD_SETLK:
>>>> case F_OFD_SETLKW:
>>>> ret = get_compat_flock64(&f, compat_ptr(arg));
>>>> if (ret != 0)
>>>> break;
>>>> old_fs = get_fs();
>>>> set_fs(KERNEL_DS);
>>>> conv_cmd = convert_fcntl_cmd(cmd);
>>>> ret = sys_fcntl(fd, conv_cmd, (unsigned long)&f);
>>>> set_fs(old_fs);
>>>> if ((conv_cmd == F_GETLK || conv_cmd == F_OFD_GETLK) && ret == 0) {
>>>> /* need to return lock information - see above for commentary */
>>>> if (f.l_start > COMPAT_LOFF_T_MAX)
>>>> ret = -EOVERFLOW;
>>>> if (f.l_len > COMPAT_LOFF_T_MAX)
>>>> f.l_len = COMPAT_LOFF_T_MAX;
>>>> if (ret == 0)
>>>> ret = put_compat_flock64(&f, compat_ptr(arg));
>>>> }
>>>> break;
>>>> [...]
>>>>
>>>> Which is similar to 4.14, however COMPAT_LOFF_T_MAX is defined as
>>>> 0x7fffffffffffffffL for all architectures and l_start/l_len are
>>>> defined as __kernel_long_t (which for a 64 bits kernel is 'long').
>>>> So the tests are not actually checking for overflow.
>>> Yes. They do not test for overflowing 32bit long. But as fcntl64 with command F_OFD_GETLK assumes 64bit long, it is fine?
>>>>
>>>> Recent kernels do the right overflow check by using COMPAT_OFF_T_MAX
>>>> with a correct value (0x7fffffff on x86). It seems to be fixed by
>>>> 80f0cce6aadebf6caf74d1f8ceb4b008ca72a9e9 (v4.12).
>>>>
>>> Sure? This commit introduces the following implementation and I think those checks are the same as before:
>>> COMPAT_SYSCALL_DEFINE3(fcntl64,...
>>
>> Not sure, I just try to see if there a kernel change by inspecting the
>> kernel patches.
>>
>>> ...
>>> case F_GETLK:
>>> ... if (f.l_start > COMPAT_OFF_T_MAX)
>>> ret = -EOVERFLOW;
>>> ...
>>> case F_OFD_GETLK:
>>> ... if (f.l_start > COMPAT_LOFF_T_MAX)
>>> ret = -EOVERFLOW;
>>>
>>> In recent kernels, only the command F_GETLK is using fixup_compat_flock() which checks against COMPAT_OFF_T_MAX.
>>>
>>> The errno=EOVERFLOW for the second fcntl call in tst-ofdlocks.c is generated by the new glibc check, but not from the kernel.
>>
>> This is the expected behaviour, the new fcntl symbol for non default LFS
>> ABI returns EOVERFLOW.
>>
>>> I've got errno=EOVERFLOW with my kernel 4.14 due to the call of fixup_compat_flock.
>>> I've also retested it on a system with kernel 4.10. There the kernel does not return with errno=EOVERFLOW.
>>>
>>
>> Can you verify that for s390-32 tst-ofdlocks-compat.c is a really using the
>> old version (i.e passing the arguments to kernel unmodified)? Otherwise
>> I am not sure if this is a glibc bug.
>>
> Yes it is. I'm running the s390-32 tst-ofdlocks-compat testcase in 64bit gdb and we are here:
> 72 TEST_VERIFY (fcntl (fd, F_OFD_GETLK, &lck) == 0);
>
> (gdb) p sizeof (lck)
> $2 = 32
>
> (gdb) p/x lck
> $3 = {l_type = 0x1, l_whence = 0x0, l_start = 0x7ffffbff, l_len = 0x1000,
> l_pid = 0x0}
>
> (gdb) x/4gx &lck
> 0x7fffeae8: 0x0001000000000000 0x000000007ffffbff
> 0x7fffeaf8: 0x0000000000001000 0x0000000000000000
>
> Stepping to syscall: ...
>
> (gdb) where
> #0 __fcntl64_nocancel_adjusted (fd=fd@entry=4, cmd=cmd@entry=36, arg=arg@entry=0x7fffeae8)
> at ../sysdeps/unix/sysv/linux/fcntl_nocancel.c:64
> #1 0x7df1e076 in __GI___libc_fcntl64 (fd=4, cmd=36) at ../sysdeps/unix/sysv/linux/fcntl64.c:51
> #2 0x7df1e006 in __old_libc_fcntl64 (fd=<optimized out>, cmd=<optimized out>) at ../sysdeps/unix/sysv/linux/fcntl.c:114
> #3 0x00401a98 in do_test () at ../sysdeps/unix/sysv/linux/tst-ofdlocks-compat.c:72
> #4 0x00401f42 in run_test_function (config=0x7fffebe8, config=0x7fffebe8, argv=0x7fffed48, argc=<optimized out>)
> at support_test_main.c:156
> #5 support_test_main (argc=<optimized out>, argv=0x7fffed48, config=config@entry=0x7fffebe8) at support_test_main.c:322
> #6 0x004017b0 in main (argc=<optimized out>, argv=<optimized out>) at ../support/test-driver.c:168
>
> We are just before the syscall:
> >│0x7df25fe0 <__fcntl64_nocancel_adjusted+32> svc 221
>
> (gdb) x/4gx arg
> 0x7fffeae8: 0x0001000000000000 0x000000007ffffbff
> 0x7fffeaf8: 0x0000000000001000 0x0000000000000000
>
> (gdb) si
> (gdb) ir 2
> r2 0xffffffffffffffb5 18446744073709551541
>
> (gdb) x/4gx arg
> 0x7fffeae8: 0x0001000000000000 0x000000007ffffbff
> 0x7fffeaf8: 0x0000000000001000 0x0000000000000000
>
Right, so we are sure the test is actually doing what is intending to do.
Now back to kernel, afaiu commit 4d2dc2cc76 does reference it fixes 94073ad77fff2:
---
fcntl: don't cap l_start and l_end values for F_GETLK64 in compat syscall
Currently, we're capping the values too low in the F_GETLK64 case. The
fields in that structure are 64-bit values, so we shouldn't need to do
any sort of fixup there.
Make sure we check that assumption at build time in the future however
by ensuring that the sizes we're copying will fit.
With this, we no longer need COMPAT_LOFF_T_MAX either, so remove it.
Fixes: 94073ad77fff2 (fs/locks: don't mess with the address limit in compat_fcntl64)
---
And 94073ad77fff2 does add a fixup_compat_flock check for F_OFD_GETLK (not
only for F_GETLK64):
---
+ case F_GETLK64:
+ case F_OFD_GETLK:
+ err = get_compat_flock64(&flock, compat_ptr(arg));
+ if (err)
+ break;
+ err = fcntl_getlk(f.file, convert_fcntl_cmd(cmd), &flock);
+ if (err)
+ break;
+ err = fixup_compat_flock(&flock);
+ if (err)
+ return err;
+ err = put_compat_flock64(&flock, compat_ptr(arg));
+ break;
---
In any way, it is still a kernel issue because prior 94073ad77fff2 F_OFD_GETLK
were handle as:
---
case F_GETLK64:
case F_SETLK64:
case F_SETLKW64:
case F_OFD_GETLK:
case F_OFD_SETLK:
case F_OFD_SETLKW:
ret = get_compat_flock64(&f, compat_ptr(arg));
if (ret != 0)
break;
old_fs = get_fs();
set_fs(KERNEL_DS);
conv_cmd = convert_fcntl_cmd(cmd);
ret = sys_fcntl(fd, conv_cmd, (unsigned long)&f);
set_fs(old_fs);
if ((conv_cmd == F_GETLK || conv_cmd == F_OFD_GETLK) && ret == 0) {
/* need to return lock information - see above for commentary */
if (f.l_start > COMPAT_LOFF_T_MAX)
ret = -EOVERFLOW;
if (f.l_len > COMPAT_LOFF_T_MAX)
f.l_len = COMPAT_LOFF_T_MAX;
if (ret == 0)
ret = put_compat_flock64(&f, compat_ptr(arg));
}
break;
---
And COMPAT_LOFF_T_MAX was defined as 0x7fffffffffffffffL for all architectures.
So it seems that kernel between 4.13 through 4.15 have this issue with for
compat kernels and I do think it is a kernel issue because fcntl64 is the
expected way to use OFD locks. GLIBC returns EOVERFLOW because from
application standpoint, it should use LFS variant instead.
On 07/09/2018 08:37 PM, Adhemerval Zanella wrote:
>
>
> On 09/07/2018 11:56, Stefan Liebler wrote:
>> On 07/09/2018 04:20 PM, Adhemerval Zanella wrote:
>>>
>>>
>>> On 09/07/2018 10:44, Stefan Liebler wrote:
>>>> On 07/06/2018 04:45 PM, Adhemerval Zanella wrote:
>>>>>
>>>>>
>>>>> On 06/07/2018 11:09, Adhemerval Zanella wrote:
>>>>>>
>>>>>>
>>>>>> On 06/07/2018 10:00, Stefan Liebler wrote:
>>>>>>> On 06/20/2018 11:43 PM, Adhemerval Zanella wrote:
>>>>>>>> Changes from previous version:
>>>>>>>>
>>>>>>>> - Add a testcase for compat fcntl using OFD locks.
>>>>>>>>
>>>>>>>> ---
>>>>>>>>
>>>>>>>> This patch fixes the OFD ("file private") locks for architectures that
>>>>>>>> support non-LFS flock definition (__USE_FILE_OFFSET64 not defined). The
>>>>>>>> issue in this case is both F_OFD_{GETLK,SETLK,SETLKW} and
>>>>>>>> F_{SET,GET}L{W}K64 expects a flock64 argument and when using old
>>>>>>>> F_OFD_* flags with a non LFS flock argument the kernel might interpret
>>>>>>>> the underlying data wrongly. Kernel idea originally was to avoid using
>>>>>>>> such flags in non-LFS syscall, but since GLIBC uses fcntl with LFS
>>>>>>>> semantic as default it is possible to provide the functionality and
>>>>>>>> avoid the bogus struct kernel passing by adjusting the struct manually
>>>>>>>> for the required flags.
>>>>>>>>
>>>>>>>> The idea follows other LFS interfaces that provide two symbols:
>>>>>>>>
>>>>>>>> 1. A new LFS fcntl64 is added on default ABI with the usual macros to
>>>>>>>> select it for FILE_OFFSET_BITS=64.
>>>>>>>>
>>>>>>>> 2. The Linux non-LFS fcntl use a stack allocated struct flock64 for
>>>>>>>> F_OFD_{GETLK,SETLK,SETLKW} copy the results on the user provided
>>>>>>>> struct.
>>>>>>>>
>>>>>>>> 3. Keep a compat symbol with old broken semantic for architectures
>>>>>>>> that do not define __OFF_T_MATCHES_OFF64_T.
>>>>>>>>
>>>>>>>> So for architectures which defines __USE_FILE_OFFSET64, fcntl64 will
>>>>>>>> aliased to fcntl and no adjustment would be required. So to actually
>>>>>>>> use F_OFD_* with LFS support the source must be built with LFS support
>>>>>>>> (_FILE_OFFSET_BITS=64).
>>>>>>>>
>>>>>>>> Also F_OFD_SETLKW command is handled a cancellation point, as for
>>>>>>>> F_SETLKW{64}.
>>>>>>>>
>>>>>>>> Checked on x86_64-linux-gnu and i686-linux-gnu.
>>>>>>>>
>>>>>>> ...
>>>>>>>
>>>>>>> Hi Adhemerval,
>>>>>>>
>>>>>>> I'm running the new test misc/tst-ofdlocks-compat on s390-32.
>>>>>>> If I run it on linux 4.17, the test succeeds and after the second fcntl call which returns zero, lck contains the region from the first fcntl call:
>>>>>>> (gdb) p/x lck
>>>>>>> $2 = {l_type = 0x1, l_whence = 0x0, l_start = 0x800003ff, l_len = 0x400, l_pid = 0xffffffff}
>>>>>>>
>>>>>>> If I run it on linux 4.14, the test fails. There the second fcntl returns -1 and errno = EOVERFLOW
>>>>>>> In this case, lck is not updated:
>>>>>>> p/x lck
>>>>>>> $4 = {l_type = 0x1, l_whence = 0x0, l_start = 0x7ffffbff, l_len = 0x1000, l_pid = 0x0}
>>>>>>>
>>>>>>> In both cases struct flock64 is just passed to syscall fcntl64 via __old_libc_fcntl64.
>>>>>>>
>>>>>>> Are the different behaviours related to a change in kernel-code?
>>>>>>
>>>>>> I think it is due the patch 'fcntl: don't cap l_start and l_end values
>>>>>> for F_GETLK64 in compat syscall' (4d2dc2cc766c3b51929658cacbc6e34fc8e242fb
>>>>>> added on v4.15). Previously for COMPAT_SYSCALL_DEFINE3(fcntl64,...) kernel
>>>>>> did:
>>>>>>
>>>>>> static long do_compat_fcntl64(unsigned int fd, unsigned int cmd,
>>>>>> compat_ulong_t arg)
>>>>>> [...]
>>>>>> case F_GETLK64:
>>>>>> case F_OFD_GETLK:
>>>>>> err = get_compat_flock64(&flock, compat_ptr(arg));
>>>>>> if (err)
>>>>>> break;
>>>>>> err = fixup_compat_flock(&flock);
>>>>>> if (err)
>>>>>> return err;
>>>>>> err = put_compat_flock64(&flock, compat_ptr(arg));
>>>>>> break;
>>>>>> [....]
>>>>>>
>>>>>> It means that even if the fcntl(..., F_OFD_GETLK, struct flock64) did not
>>>>>> fail, kernel will return EOVERFLOW due 'fixup_compat_flock'. The patch
>>>>>> changed to:
>>>>>>
>>>>>> [...]
>>>>>> case F_GETLK64:
>>>>>> case F_OFD_GETLK:
>>>>>> err = get_compat_flock64(&flock, compat_ptr(arg));
>>>>>> if (!err)
>>>>>> err= put_compat_flock64(&flock, compat_ptr(arg));
>>>>>> break;
>>>>>> [....]
>>>>>>
>>>> Yes, this sounds reasonable.
>>>> It was introduced with commit 'fs/locks: don't mess with the address limit in compat_fcntl64' (94073ad77fff221b5e66b8b9863a546ba212d6a3)
>>>>
>>>>>> So if compat fcntl will just return if get_compat_flock64 succeed. Also
>>>>>> afaiu only compat syscall is affected by it I think, the generic OFD
>>>>>> locks for 32 bits go through fcntl_getlk64 and it does return EOVERFLOW
>>>>>> in the aforementioned case.
>>>>>>
>>>>>> I am not sure which would be the best way to get around this kernel
>>>>>> issue, any suggestion to harden the testcase? It also suggest to me that
>>>>>> the possible usercase I assume in my testcase (OFD locks with struct
>>>>>> flock64) never really worked on previous releases...
>>>>>>
>>>>>
>>>>> Also, on x86 4.4 where actually tested the kernel OFD locks does:
>>>>>
>>>>> fs/compat.c:
>>>>> [...]
>>>>> case F_GETLK64:
>>>>> case F_SETLK64:
>>>>> case F_SETLKW64:
>>>>> case F_OFD_GETLK:
>>>>> case F_OFD_SETLK:
>>>>> case F_OFD_SETLKW:
>>>>> ret = get_compat_flock64(&f, compat_ptr(arg));
>>>>> if (ret != 0)
>>>>> break;
>>>>> old_fs = get_fs();
>>>>> set_fs(KERNEL_DS);
>>>>> conv_cmd = convert_fcntl_cmd(cmd);
>>>>> ret = sys_fcntl(fd, conv_cmd, (unsigned long)&f);
>>>>> set_fs(old_fs);
>>>>> if ((conv_cmd == F_GETLK || conv_cmd == F_OFD_GETLK) && ret == 0) {
>>>>> /* need to return lock information - see above for commentary */
>>>>> if (f.l_start > COMPAT_LOFF_T_MAX)
>>>>> ret = -EOVERFLOW;
>>>>> if (f.l_len > COMPAT_LOFF_T_MAX)
>>>>> f.l_len = COMPAT_LOFF_T_MAX;
>>>>> if (ret == 0)
>>>>> ret = put_compat_flock64(&f, compat_ptr(arg));
>>>>> }
>>>>> break;
>>>>> [...]
>>>>>
>>>>> Which is similar to 4.14, however COMPAT_LOFF_T_MAX is defined as
>>>>> 0x7fffffffffffffffL for all architectures and l_start/l_len are
>>>>> defined as __kernel_long_t (which for a 64 bits kernel is 'long').
>>>>> So the tests are not actually checking for overflow.
>>>> Yes. They do not test for overflowing 32bit long. But as fcntl64 with command F_OFD_GETLK assumes 64bit long, it is fine?
>>>>>
>>>>> Recent kernels do the right overflow check by using COMPAT_OFF_T_MAX
>>>>> with a correct value (0x7fffffff on x86). It seems to be fixed by
>>>>> 80f0cce6aadebf6caf74d1f8ceb4b008ca72a9e9 (v4.12).
>>>>>
>>>> Sure? This commit introduces the following implementation and I think those checks are the same as before:
>>>> COMPAT_SYSCALL_DEFINE3(fcntl64,...
>>>
>>> Not sure, I just try to see if there a kernel change by inspecting the
>>> kernel patches.
>>>
>>>> ...
>>>> case F_GETLK:
>>>> ... if (f.l_start > COMPAT_OFF_T_MAX)
>>>> ret = -EOVERFLOW;
>>>> ...
>>>> case F_OFD_GETLK:
>>>> ... if (f.l_start > COMPAT_LOFF_T_MAX)
>>>> ret = -EOVERFLOW;
>>>>
>>>> In recent kernels, only the command F_GETLK is using fixup_compat_flock() which checks against COMPAT_OFF_T_MAX.
>>>>
>>>> The errno=EOVERFLOW for the second fcntl call in tst-ofdlocks.c is generated by the new glibc check, but not from the kernel.
>>>
>>> This is the expected behaviour, the new fcntl symbol for non default LFS
>>> ABI returns EOVERFLOW.
>>>
>>>> I've got errno=EOVERFLOW with my kernel 4.14 due to the call of fixup_compat_flock.
>>>> I've also retested it on a system with kernel 4.10. There the kernel does not return with errno=EOVERFLOW.
>>>>
>>>
>>> Can you verify that for s390-32 tst-ofdlocks-compat.c is a really using the
>>> old version (i.e passing the arguments to kernel unmodified)? Otherwise
>>> I am not sure if this is a glibc bug.
>>>
>> Yes it is. I'm running the s390-32 tst-ofdlocks-compat testcase in 64bit gdb and we are here:
>> 72 TEST_VERIFY (fcntl (fd, F_OFD_GETLK, &lck) == 0);
>>
>> (gdb) p sizeof (lck)
>> $2 = 32
>>
>> (gdb) p/x lck
>> $3 = {l_type = 0x1, l_whence = 0x0, l_start = 0x7ffffbff, l_len = 0x1000,
>> l_pid = 0x0}
>>
>> (gdb) x/4gx &lck
>> 0x7fffeae8: 0x0001000000000000 0x000000007ffffbff
>> 0x7fffeaf8: 0x0000000000001000 0x0000000000000000
>>
>> Stepping to syscall: ...
>>
>> (gdb) where
>> #0 __fcntl64_nocancel_adjusted (fd=fd@entry=4, cmd=cmd@entry=36, arg=arg@entry=0x7fffeae8)
>> at ../sysdeps/unix/sysv/linux/fcntl_nocancel.c:64
>> #1 0x7df1e076 in __GI___libc_fcntl64 (fd=4, cmd=36) at ../sysdeps/unix/sysv/linux/fcntl64.c:51
>> #2 0x7df1e006 in __old_libc_fcntl64 (fd=<optimized out>, cmd=<optimized out>) at ../sysdeps/unix/sysv/linux/fcntl.c:114
>> #3 0x00401a98 in do_test () at ../sysdeps/unix/sysv/linux/tst-ofdlocks-compat.c:72
>> #4 0x00401f42 in run_test_function (config=0x7fffebe8, config=0x7fffebe8, argv=0x7fffed48, argc=<optimized out>)
>> at support_test_main.c:156
>> #5 support_test_main (argc=<optimized out>, argv=0x7fffed48, config=config@entry=0x7fffebe8) at support_test_main.c:322
>> #6 0x004017b0 in main (argc=<optimized out>, argv=<optimized out>) at ../support/test-driver.c:168
>>
>> We are just before the syscall:
>> >│0x7df25fe0 <__fcntl64_nocancel_adjusted+32> svc 221
>>
>> (gdb) x/4gx arg
>> 0x7fffeae8: 0x0001000000000000 0x000000007ffffbff
>> 0x7fffeaf8: 0x0000000000001000 0x0000000000000000
>>
>> (gdb) si
>> (gdb) ir 2
>> r2 0xffffffffffffffb5 18446744073709551541
>>
>> (gdb) x/4gx arg
>> 0x7fffeae8: 0x0001000000000000 0x000000007ffffbff
>> 0x7fffeaf8: 0x0000000000001000 0x0000000000000000
>>
>
> Right, so we are sure the test is actually doing what is intending to do.
> Now back to kernel, afaiu commit 4d2dc2cc76 does reference it fixes 94073ad77fff2:
>
> ---
> fcntl: don't cap l_start and l_end values for F_GETLK64 in compat syscall
>
> Currently, we're capping the values too low in the F_GETLK64 case. The
> fields in that structure are 64-bit values, so we shouldn't need to do
> any sort of fixup there.
>
> Make sure we check that assumption at build time in the future however
> by ensuring that the sizes we're copying will fit.
>
> With this, we no longer need COMPAT_LOFF_T_MAX either, so remove it.
>
> Fixes: 94073ad77fff2 (fs/locks: don't mess with the address limit in compat_fcntl64)
> ---
>
> And 94073ad77fff2 does add a fixup_compat_flock check for F_OFD_GETLK (not
> only for F_GETLK64):
>
> ---
> + case F_GETLK64:
> + case F_OFD_GETLK:
> + err = get_compat_flock64(&flock, compat_ptr(arg));
> + if (err)
> + break;
> + err = fcntl_getlk(f.file, convert_fcntl_cmd(cmd), &flock);
> + if (err)
> + break;
> + err = fixup_compat_flock(&flock);
> + if (err)
> + return err;
> + err = put_compat_flock64(&flock, compat_ptr(arg));
> + break;
> ---
>
> In any way, it is still a kernel issue because prior 94073ad77fff2 F_OFD_GETLK
> were handle as:
>
> ---
> case F_GETLK64:
> case F_SETLK64:
> case F_SETLKW64:
> case F_OFD_GETLK:
> case F_OFD_SETLK:
> case F_OFD_SETLKW:
> ret = get_compat_flock64(&f, compat_ptr(arg));
> if (ret != 0)
> break;
> old_fs = get_fs();
> set_fs(KERNEL_DS);
> conv_cmd = convert_fcntl_cmd(cmd);
> ret = sys_fcntl(fd, conv_cmd, (unsigned long)&f);
> set_fs(old_fs);
> if ((conv_cmd == F_GETLK || conv_cmd == F_OFD_GETLK) && ret == 0) {
> /* need to return lock information - see above for commentary */
> if (f.l_start > COMPAT_LOFF_T_MAX)
> ret = -EOVERFLOW;
> if (f.l_len > COMPAT_LOFF_T_MAX)
> f.l_len = COMPAT_LOFF_T_MAX;
> if (ret == 0)
> ret = put_compat_flock64(&f, compat_ptr(arg));
> }
> break;
> ---
>
> And COMPAT_LOFF_T_MAX was defined as 0x7fffffffffffffffL for all architectures.
>
> So it seems that kernel between 4.13 through 4.15 have this issue with for
> compat kernels and I do think it is a kernel issue because fcntl64 is the
> expected way to use OFD locks. GLIBC returns EOVERFLOW because from
> application standpoint, it should use LFS variant instead.
>
Yes, I agree with you.
Shall we document this kernel issue in the release-wiki and/or the
testcase itself?
@@ -110,6 +110,12 @@ Deprecated and removed features, and other changes affecting compatibility:
restriction (rejecting '_' in host names, among other things) has been
removed, for increased compatibility with non-IDN name resolution.
+* The fcntl function now have a Long File Support variant named fcntl64. It
+ is added to fix some Linux Open File Description (OFD) locks usage on non
+ LFS mode. As for others *64 functions, fcntl64 semantics are analogous with
+ fcntl and LFS support is handled transparently. Also for Linux, the OFD
+ locks act as a cancellation entrypoint.
+
Changes to build and runtime requirements:
[Add changes to build and runtime requirements here]
@@ -39,7 +39,7 @@
static void
check_one_fd (int fd, int mode)
{
- if (__builtin_expect (__fcntl_nocancel (fd, F_GETFD), 0) == -1
+ if (__builtin_expect (__fcntl64_nocancel (fd, F_GETFD), 0) == -1
&& errno == EBADF)
{
const char *name;
@@ -10,11 +10,16 @@ extern int __libc_open (const char *file, int oflag, ...);
libc_hidden_proto (__libc_open)
extern int __libc_fcntl (int fd, int cmd, ...);
libc_hidden_proto (__libc_fcntl)
-extern int __fcntl_nocancel_adjusted (int fd, int cmd, void *arg) attribute_hidden;
+extern int __fcntl64_nocancel_adjusted (int fd, int cmd, void *arg)
+ attribute_hidden;
+extern int __libc_fcntl64 (int fd, int cmd, ...);
+libc_hidden_proto (__libc_fcntl64)
extern int __open (const char *__file, int __oflag, ...);
libc_hidden_proto (__open)
extern int __fcntl (int __fd, int __cmd, ...);
libc_hidden_proto (__fcntl)
+extern int __fcntl64 (int __fd, int __cmd, ...) attribute_hidden;
+libc_hidden_proto (__fcntl64)
extern int __openat (int __fd, const char *__file, int __oflag, ...)
__nonnull ((2));
libc_hidden_proto (__openat)
@@ -40,7 +40,7 @@ routines := \
mkdir mkdirat \
open open_2 open64 open64_2 openat openat_2 openat64 openat64_2 \
read write lseek lseek64 access euidaccess faccessat \
- fcntl flock lockf lockf64 \
+ fcntl fcntl64 flock lockf lockf64 \
close dup dup2 dup3 pipe pipe2 \
creat creat64 \
chdir fchdir \
@@ -89,6 +89,7 @@ CFLAGS-open64.c += -fexceptions -fasynchronous-unwind-tables
CFLAGS-creat.c += -fexceptions -fasynchronous-unwind-tables
CFLAGS-creat64.c += -fexceptions -fasynchronous-unwind-tables
CFLAGS-fcntl.c += -fexceptions -fasynchronous-unwind-tables
+CFLAGS-fcntl64.c += -fexceptions -fasynchronous-unwind-tables
CFLAGS-poll.c += -fexceptions -fasynchronous-unwind-tables
CFLAGS-ppoll.c += -fexceptions -fasynchronous-unwind-tables
CFLAGS-lockf.c += -fexceptions
@@ -128,8 +128,11 @@ libc {
GLIBC_2.27 {
copy_file_range;
}
+ GLIBC_2.28 {
+ fcntl64;
+ }
GLIBC_PRIVATE {
- __libc_fcntl;
+ __libc_fcntl64;
__fcntl_nocancel;
__open64_nocancel;
__write_nocancel;
@@ -167,7 +167,18 @@ typedef __pid_t pid_t;
This function is a cancellation point and therefore not marked with
__THROW. */
+#ifndef __USE_FILE_OFFSET64
extern int fcntl (int __fd, int __cmd, ...);
+#else
+# ifdef __REDIRECT
+extern int __REDIRECT (fcntl, (int __fd, int __cmd, ...), fcntl64);
+# else
+# define fcntl fcntl64
+# endif
+#endif
+#ifdef __USE_LARGEFILE64
+extern int fcntl64 (int __fd, int __cmd, ...);
+#endif
/* Open FILE and return a new file descriptor for it, or -1 on error.
OFLAG determines the type of access used. If O_CREAT or O_TMPFILE is set
new file mode 100644
@@ -0,0 +1,38 @@
+/* Manipulate file descriptor. Stub LFS version.
+ Copyright (C) 2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <errno.h>
+#include <fcntl.h>
+
+/* Perform file control operations on FD. */
+int
+__fcntl64 (int fd, int cmd, ...)
+{
+ if (fd < 0)
+ {
+ __set_errno (EBADF);
+ return -1;
+ }
+
+ __set_errno (ENOSYS);
+ return -1;
+}
+libc_hidden_def (__fcntl64)
+stub_warning (fcntl64)
+
+weak_alias (__fcntl64, fcntl64)
@@ -82,7 +82,7 @@ static void timeout_handler (int signum) {};
memset (&fl, '\0', sizeof (struct flock)); \
fl.l_type = (type); \
fl.l_whence = SEEK_SET; \
- if (__fcntl_nocancel ((fd), F_SETLKW, &fl) < 0)
+ if (__fcntl64_nocancel ((fd), F_SETLKW, &fl) < 0)
#define LOCKING_FAILED() \
goto unalarm_return
@@ -90,7 +90,7 @@ static void timeout_handler (int signum) {};
#define UNLOCK_FILE(fd) \
/* Unlock the file. */ \
fl.l_type = F_UNLCK; \
- __fcntl_nocancel ((fd), F_SETLKW, &fl); \
+ __fcntl64_nocancel ((fd), F_SETLKW, &fl); \
\
unalarm_return: \
/* Reset the signal handler and alarm. We must reset the alarm \
@@ -3281,12 +3281,13 @@ Set process or process group ID to receive @code{SIGIO} signals.
@xref{Interrupt Input}.
@end vtable
-This function is a cancellation point in multi-threaded programs. This
-is a problem if the thread allocates some resources (like memory, file
-descriptors, semaphores or whatever) at the time @code{fcntl} is
-called. If the thread gets canceled these resources stay allocated
-until the program ends. To avoid this calls to @code{fcntl} should be
-protected using cancellation handlers.
+This function is a cancellation point in multi-threaded programs for the
+commands @code{F_SETLKW} (and the LFS analogous @code{F_SETLKW64}) and
+@code {F_OFD_SETLKW}. This is a problem if the thread allocates some
+resources (like memory, file descriptors, semaphores or whatever) at the time
+@code{fcntl} is called. If the thread gets canceled these resources stay
+allocated until the program ends. To avoid this calls to @code{fcntl} should
+be protected using cancellation handlers.
@c ref pthread_cleanup_push / pthread_cleanup_pop
@end deftypefun
@@ -191,6 +191,7 @@ CFLAGS-sem_timedwait.c += -fexceptions -fasynchronous-unwind-tables
# These are the function wrappers we have to duplicate here.
CFLAGS-fcntl.c += -fexceptions -fasynchronous-unwind-tables
+CFLAGS-fcntl64.c += -fexceptions -fasynchronous-unwind-tables
CFLAGS-lockf.c += -fexceptions
CFLAGS-pread.c += -fexceptions -fasynchronous-unwind-tables
CFLAGS-pread64.c += -fexceptions -fasynchronous-unwind-tables
@@ -210,3 +210,8 @@ libc_hidden_def (__libc_fcntl)
weak_alias (__libc_fcntl, __fcntl)
libc_hidden_weak (__fcntl)
weak_alias (__libc_fcntl, fcntl)
+
+strong_alias (__libc_fcntl, __libc_fcntl64)
+libc_hidden_def (__libc_fcntl64)
+weak_alias (__libc_fcntl64, __fcntl64)
+libc_hidden_weak (__fcntl64)
@@ -2033,6 +2033,8 @@ GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
GLIBC_2.27 wcstof64x F
GLIBC_2.27 wcstof64x_l F
+GLIBC_2.28 fcntl F
+GLIBC_2.28 fcntl64 F
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
@@ -38,7 +38,7 @@ __fdopendir (int fd)
}
/* Make sure the descriptor allows for reading. */
- int flags = __fcntl_nocancel (fd, F_GETFL);
+ int flags = __fcntl64_nocancel (fd, F_GETFL);
if (__glibc_unlikely (flags == -1))
return NULL;
if (__glibc_unlikely ((flags & O_ACCMODE) == O_WRONLY))
@@ -99,7 +99,7 @@ __alloc_dir (int fd, bool close_fd, int flags, const struct stat64 *statp)
/* We have to set the close-on-exit flag if the user provided the
file descriptor. */
if (!close_fd
- && __glibc_unlikely (__fcntl_nocancel (fd, F_SETFD, FD_CLOEXEC) < 0))
+ && __glibc_unlikely (__fcntl64_nocancel (fd, F_SETFD, FD_CLOEXEC) < 0))
goto lose;
const size_t default_allocation = (4 * BUFSIZ < sizeof (struct dirent64)
@@ -37,7 +37,7 @@ fcntl_compat (int fd, int cmd, ...)
va_start (ap, cmd);
arg = va_arg (ap, void *);
va_end (ap);
- return __libc_fcntl (fd, cmd, arg);
+ return __libc_fcntl64 (fd, cmd, arg);
}
weak_alias (fcntl_compat, fcntl_alias)
@@ -45,7 +45,9 @@ sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \
tests += tst-clone tst-clone2 tst-clone3 tst-fanotify tst-personality \
tst-quota tst-sync_file_range tst-sysconf-iov_max tst-ttyname \
test-errno-linux tst-memfd_create tst-mlock2 tst-pkey \
- tst-rlimit-infinity
+ tst-rlimit-infinity tst-ofdlocks
+tests-internal += tst-ofdlocks-compat
+
# Generate the list of SYS_* macros for the system calls (__NR_*
# macros). The file syscall-names.list contains all possible system
@@ -2131,3 +2131,4 @@ GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
GLIBC_2.27 wcstof64x F
GLIBC_2.27 wcstof64x_l F
+GLIBC_2.28 fcntl64 F
@@ -2026,6 +2026,7 @@ GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
GLIBC_2.27 wcstof64x F
GLIBC_2.27 wcstof64x_l F
+GLIBC_2.28 fcntl64 F
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
@@ -115,6 +115,8 @@ GLIBC_2.27 wcstof32x F
GLIBC_2.27 wcstof32x_l F
GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
+GLIBC_2.28 fcntl F
+GLIBC_2.28 fcntl64 F
GLIBC_2.4 _Exit F
GLIBC_2.4 _IO_2_1_stderr_ D 0xa0
GLIBC_2.4 _IO_2_1_stdin_ D 0xa0
@@ -20,15 +20,12 @@
#include <stdarg.h>
#include <errno.h>
#include <sysdep-cancel.h>
-#include <not-cancel.h>
-#ifndef __NR_fcntl64
-# define __NR_fcntl64 __NR_fcntl
-#endif
+#ifndef __OFF_T_MATCHES_OFF64_T
-#ifndef FCNTL_ADJUST_CMD
-# define FCNTL_ADJUST_CMD(__cmd) __cmd
-#endif
+# ifndef FCNTL_ADJUST_CMD
+# define FCNTL_ADJUST_CMD(__cmd) __cmd
+# endif
int
__libc_fcntl (int fd, int cmd, ...)
@@ -42,13 +39,83 @@ __libc_fcntl (int fd, int cmd, ...)
cmd = FCNTL_ADJUST_CMD (cmd);
- if (cmd == F_SETLKW || cmd == F_SETLKW64)
- return SYSCALL_CANCEL (fcntl64, fd, cmd, (void *) arg);
-
- return __fcntl_nocancel_adjusted (fd, cmd, arg);
+ switch (cmd)
+ {
+ case F_SETLKW:
+ case F_SETLKW64:
+ return SYSCALL_CANCEL (fcntl64, fd, cmd, arg);
+ case F_OFD_SETLKW:
+ {
+ struct flock *flk = (struct flock *) arg;
+ struct flock64 flk64 =
+ {
+ .l_type = flk->l_type,
+ .l_whence = flk->l_whence,
+ .l_start = flk->l_start,
+ .l_len = flk->l_len,
+ .l_pid = flk->l_pid
+ };
+ return SYSCALL_CANCEL (fcntl64, fd, cmd, &flk64);
+ }
+ case F_OFD_GETLK:
+ case F_OFD_SETLK:
+ {
+ struct flock *flk = (struct flock *) arg;
+ struct flock64 flk64 =
+ {
+ .l_type = flk->l_type,
+ .l_whence = flk->l_whence,
+ .l_start = flk->l_start,
+ .l_len = flk->l_len,
+ .l_pid = flk->l_pid
+ };
+ int ret = INLINE_SYSCALL_CALL (fcntl64, fd, cmd, &flk64);
+ if (ret == -1)
+ return -1;
+ if ((off_t) flk64.l_start != flk64.l_start
+ || (off_t) flk64.l_len != flk64.l_len)
+ {
+ __set_errno (EOVERFLOW);
+ return -1;
+ }
+ flk->l_type = flk64.l_type;
+ flk->l_whence = flk64.l_whence;
+ flk->l_start = flk64.l_start;
+ flk->l_len = flk64.l_len;
+ flk->l_pid = flk64.l_pid;
+ return ret;
+ }
+ /* case F_OFD_GETLK:
+ case F_OFD_GETLK64:
+ case F_SETLK64:
+ case F_GETOWN: */
+ default:
+ return __fcntl64_nocancel_adjusted (fd, cmd, arg);
+ }
}
libc_hidden_def (__libc_fcntl)
weak_alias (__libc_fcntl, __fcntl)
libc_hidden_weak (__fcntl)
+
+# include <shlib-compat.h>
+# if SHLIB_COMPAT(libc, GLIBC_2_0, GLIBC_2_28)
+int
+__old_libc_fcntl64 (int fd, int cmd, ...)
+{
+ va_list ap;
+ void *arg;
+
+ va_start (ap, cmd);
+ arg = va_arg (ap, void *);
+ va_end (ap);
+
+ return __libc_fcntl64 (fd, cmd, arg);
+}
+compat_symbol (libc, __old_libc_fcntl64, fcntl, GLIBC_2_0);
+versioned_symbol (libc, __libc_fcntl, fcntl, GLIBC_2_28);
+# else
weak_alias (__libc_fcntl, fcntl)
+# endif
+
+#endif /* __OFF_T_MATCHES_OFF64_T */
new file mode 100644
@@ -0,0 +1,63 @@
+/* Manipulate file descriptor. Linux LFS version.
+ Copyright (C) 2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#define fcntl __no_decl_fcntl
+#define __fcntl __no_decl___fcntl
+#include <fcntl.h>
+#undef fcntl
+#undef __fcntl
+#include <stdarg.h>
+#include <errno.h>
+#include <sysdep-cancel.h>
+
+#ifndef __NR_fcntl64
+# define __NR_fcntl64 __NR_fcntl
+#endif
+
+#ifndef FCNTL_ADJUST_CMD
+# define FCNTL_ADJUST_CMD(__cmd) __cmd
+#endif
+
+int
+__libc_fcntl64 (int fd, int cmd, ...)
+{
+ va_list ap;
+ void *arg;
+
+ va_start (ap, cmd);
+ arg = va_arg (ap, void *);
+ va_end (ap);
+
+ cmd = FCNTL_ADJUST_CMD (cmd);
+
+ if (cmd == F_SETLKW || cmd == F_SETLKW64 || cmd == F_OFD_SETLKW)
+ return SYSCALL_CANCEL (fcntl64, fd, cmd, arg);
+
+ return __fcntl64_nocancel_adjusted (fd, cmd, arg);
+}
+libc_hidden_def (__libc_fcntl64)
+weak_alias (__libc_fcntl64, __fcntl64)
+libc_hidden_weak (__fcntl64)
+weak_alias (__libc_fcntl64, fcntl64)
+
+#ifdef __OFF_T_MATCHES_OFF64_T
+weak_alias (__libc_fcntl64, __libc_fcntl)
+weak_alias (__libc_fcntl64, __fcntl)
+weak_alias (__libc_fcntl64, __GI___fcntl)
+weak_alias (__libc_fcntl64, fcntl)
+#endif
@@ -31,7 +31,7 @@
#endif
int
-__fcntl_nocancel (int fd, int cmd, ...)
+__fcntl64_nocancel (int fd, int cmd, ...)
{
va_list ap;
void *arg;
@@ -42,12 +42,12 @@ __fcntl_nocancel (int fd, int cmd, ...)
cmd = FCNTL_ADJUST_CMD (cmd);
- return __fcntl_nocancel_adjusted (fd, cmd, arg);
+ return __fcntl64_nocancel_adjusted (fd, cmd, arg);
}
-hidden_def (__fcntl_nocancel)
+hidden_def (__fcntl64_nocancel)
int
-__fcntl_nocancel_adjusted (int fd, int cmd, void *arg)
+__fcntl64_nocancel_adjusted (int fd, int cmd, void *arg)
{
if (cmd == F_GETOWN)
{
@@ -1872,6 +1872,8 @@ GLIBC_2.27 wcstof32x F
GLIBC_2.27 wcstof32x_l F
GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
+GLIBC_2.28 fcntl F
+GLIBC_2.28 fcntl64 F
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
@@ -2037,6 +2037,8 @@ GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
GLIBC_2.27 wcstof64x F
GLIBC_2.27 wcstof64x_l F
+GLIBC_2.28 fcntl F
+GLIBC_2.28 fcntl64 F
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
@@ -1907,6 +1907,7 @@ GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
GLIBC_2.27 wcstof64x F
GLIBC_2.27 wcstof64x_l F
+GLIBC_2.28 fcntl64 F
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
@@ -116,6 +116,8 @@ GLIBC_2.27 wcstof32x F
GLIBC_2.27 wcstof32x_l F
GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
+GLIBC_2.28 fcntl F
+GLIBC_2.28 fcntl64 F
GLIBC_2.4 _Exit F
GLIBC_2.4 _IO_2_1_stderr_ D 0x98
GLIBC_2.4 _IO_2_1_stdin_ D 0x98
@@ -1981,6 +1981,8 @@ GLIBC_2.27 wcstof32x F
GLIBC_2.27 wcstof32x_l F
GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
+GLIBC_2.28 fcntl F
+GLIBC_2.28 fcntl64 F
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
@@ -2122,3 +2122,5 @@ GLIBC_2.27 wcstof32x F
GLIBC_2.27 wcstof32x_l F
GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
+GLIBC_2.28 fcntl F
+GLIBC_2.28 fcntl64 F
@@ -1959,6 +1959,8 @@ GLIBC_2.27 wcstof32x F
GLIBC_2.27 wcstof32x_l F
GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
+GLIBC_2.28 fcntl F
+GLIBC_2.28 fcntl64 F
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
@@ -1957,6 +1957,8 @@ GLIBC_2.27 wcstof32x F
GLIBC_2.27 wcstof32x_l F
GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
+GLIBC_2.28 fcntl F
+GLIBC_2.28 fcntl64 F
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
@@ -1965,6 +1965,8 @@ GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
GLIBC_2.27 wcstof64x F
GLIBC_2.27 wcstof64x_l F
+GLIBC_2.28 fcntl F
+GLIBC_2.28 fcntl64 F
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
@@ -1961,6 +1961,7 @@ GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
GLIBC_2.27 wcstof64x F
GLIBC_2.27 wcstof64x_l F
+GLIBC_2.28 fcntl64 F
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
@@ -2163,3 +2163,5 @@ GLIBC_2.27 wcstof32x F
GLIBC_2.27 wcstof32x_l F
GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
+GLIBC_2.28 fcntl F
+GLIBC_2.28 fcntl64 F
@@ -76,7 +76,7 @@ __typeof (pause) __pause_nocancel;
__typeof (__nanosleep) __nanosleep_nocancel;
/* Uncancelable fcntl. */
-__typeof (__fcntl) __fcntl_nocancel;
+__typeof (__fcntl) __fcntl64_nocancel;
#if IS_IN (libc) || IS_IN (rtld)
hidden_proto (__open_nocancel)
@@ -89,7 +89,7 @@ hidden_proto (__close_nocancel)
hidden_proto (__waitpid_nocancel)
hidden_proto (__pause_nocancel)
hidden_proto (__nanosleep_nocancel)
-hidden_proto (__fcntl_nocancel)
+hidden_proto (__fcntl64_nocancel)
#endif
#endif /* NOT_CANCEL_H */
@@ -1985,6 +1985,8 @@ GLIBC_2.27 wcstof32x F
GLIBC_2.27 wcstof32x_l F
GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
+GLIBC_2.28 fcntl F
+GLIBC_2.28 fcntl64 F
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
@@ -1989,6 +1989,8 @@ GLIBC_2.27 wcstof32x F
GLIBC_2.27 wcstof32x_l F
GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
+GLIBC_2.28 fcntl F
+GLIBC_2.28 fcntl64 F
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
@@ -2221,3 +2221,4 @@ GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
GLIBC_2.27 wcstof64x F
GLIBC_2.27 wcstof64x_l F
+GLIBC_2.28 fcntl64 F
@@ -116,6 +116,7 @@ GLIBC_2.27 wcstof32x F
GLIBC_2.27 wcstof32x_l F
GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
+GLIBC_2.28 fcntl64 F
GLIBC_2.3 _Exit F
GLIBC_2.3 _IO_2_1_stderr_ D 0xe0
GLIBC_2.3 _IO_2_1_stdin_ D 0xe0
@@ -2093,3 +2093,4 @@ GLIBC_2.27 xdrstdio_create F
GLIBC_2.27 xencrypt F
GLIBC_2.27 xprt_register F
GLIBC_2.27 xprt_unregister F
+GLIBC_2.28 fcntl64 F
@@ -1994,6 +1994,8 @@ GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
GLIBC_2.27 wcstof64x F
GLIBC_2.27 wcstof64x_l F
+GLIBC_2.28 fcntl F
+GLIBC_2.28 fcntl64 F
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
@@ -1900,6 +1900,7 @@ GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
GLIBC_2.27 wcstof64x F
GLIBC_2.27 wcstof64x_l F
+GLIBC_2.28 fcntl64 F
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
@@ -1876,6 +1876,8 @@ GLIBC_2.27 wcstof32x F
GLIBC_2.27 wcstof32x_l F
GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
+GLIBC_2.28 fcntl F
+GLIBC_2.28 fcntl64 F
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
@@ -1988,6 +1988,8 @@ GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
GLIBC_2.27 wcstof64x F
GLIBC_2.27 wcstof64x_l F
+GLIBC_2.28 fcntl F
+GLIBC_2.28 fcntl64 F
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
@@ -1930,6 +1930,7 @@ GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
GLIBC_2.27 wcstof64x F
GLIBC_2.27 wcstof64x_l F
+GLIBC_2.28 fcntl64 F
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
new file mode 100644
@@ -0,0 +1,78 @@
+/* Check non representable OFD locks regions in non-LFS mode for compat
+ mode (BZ #20251)
+ Copyright (C) 2018 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or
+ modify it under the terms of the GNU General Public License
+ as published by the Free Software Foundation; either version 2
+ of the License, or (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <unistd.h>
+#include <fcntl.h>
+#include <stdint.h>
+#include <errno.h>
+
+#include <support/temp_file.h>
+#include <support/check.h>
+
+#include <shlib-compat.h>
+#if TEST_COMPAT (libc, GLIBC_2_0, GLIBC_2_28)
+compat_symbol_reference (libc, fcntl, fcntl, GLIBC_2_0);
+#endif
+
+static char *temp_filename;
+static int temp_fd;
+
+static void
+do_prepare (int argc, char **argv)
+{
+ temp_fd = create_temp_file ("tst-ofdlocks-compat.", &temp_filename);
+ TEST_VERIFY_EXIT (temp_fd != -1);
+}
+
+#define PREPARE do_prepare
+
+static int
+do_test (void)
+{
+ /* The compat fcntl version for architectures which support non-LFS
+ operations does not wrap the flock OFD argument, so the struct is passed
+ unmodified to kernel. It means no EOVERFLOW is returned, so operations
+ with LFS should not incur in failure. */
+
+ struct flock64 lck64 = {
+ .l_type = F_WRLCK,
+ .l_whence = SEEK_SET,
+ .l_start = (off64_t)INT32_MAX + 1024,
+ .l_len = 1024,
+ };
+ TEST_VERIFY_EXIT (fcntl (temp_fd, F_OFD_SETLKW, &lck64) == 0);
+
+ /* Open file description locks placed through the same open file description
+ (either by same file descriptor or a duplicated one created by fork,
+ dup, fcntl F_DUPFD, etc.) overwrites then old lock. To force a
+ conflicting lock combination, it creates a new file descriptor. */
+ int fd = open64 (temp_filename, O_RDWR, 0666);
+ TEST_VERIFY_EXIT (fd != -1);
+
+ struct flock64 lck = {
+ .l_type = F_WRLCK,
+ .l_whence = SEEK_SET,
+ .l_start = INT32_MAX - 1024,
+ .l_len = 4 * 1024,
+ };
+ TEST_VERIFY (fcntl (fd, F_OFD_GETLK, &lck) == 0);
+
+ return 0;
+}
+
+#include <support/test-driver.c>
new file mode 100644
@@ -0,0 +1,76 @@
+/* Check non representable OFD locks regions in non-LFS mode (BZ #20251)
+ Copyright (C) 2018 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or
+ modify it under the terms of the GNU General Public License
+ as published by the Free Software Foundation; either version 2
+ of the License, or (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <unistd.h>
+#include <fcntl.h>
+#include <stdint.h>
+#include <errno.h>
+
+#include <support/temp_file.h>
+#include <support/check.h>
+
+static char *temp_filename;
+static int temp_fd;
+
+static void
+do_prepare (int argc, char **argv)
+{
+ temp_fd = create_temp_file ("tst-ofdlocks.", &temp_filename);
+ TEST_VERIFY_EXIT (temp_fd != -1);
+}
+
+#define PREPARE do_prepare
+
+static int
+do_test (void)
+{
+ /* It first allocates a open file description lock range which can not
+ be represented in a 32 bit struct flock. */
+ struct flock64 lck64 = {
+ .l_type = F_WRLCK,
+ .l_whence = SEEK_SET,
+ .l_start = (off64_t)INT32_MAX + 1024,
+ .l_len = 1024,
+ };
+ TEST_VERIFY_EXIT (fcntl64 (temp_fd, F_OFD_SETLKW, &lck64) == 0);
+
+ /* Open file description locks placed through the same open file description
+ (either by same file descriptor or a duplicated one created by fork,
+ dup, fcntl F_DUPFD, etc.) overwrites then old lock. To force a
+ conflicting lock combination, it creates a new file descriptor. */
+ int fd = open64 (temp_filename, O_RDWR, 0666);
+ TEST_VERIFY_EXIT (fd != -1);
+
+ /* It tries then to allocate another open file descriptior with a valid
+ non-LFS bits struct flock but which will result in a conflicted region
+ which can not be represented in a non-LFS struct flock. */
+ struct flock lck = {
+ .l_type = F_WRLCK,
+ .l_whence = SEEK_SET,
+ .l_start = INT32_MAX - 1024,
+ .l_len = 4 * 1024,
+ };
+ int r = fcntl (fd, F_OFD_GETLK, &lck);
+ if (sizeof (off_t) != sizeof (off64_t))
+ TEST_VERIFY (r == -1 && errno == EOVERFLOW);
+ else
+ TEST_VERIFY (r == 0);
+
+ return 0;
+}
+
+#include <support/test-driver.c>
@@ -1888,6 +1888,7 @@ GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
GLIBC_2.27 wcstof64x F
GLIBC_2.27 wcstof64x_l F
+GLIBC_2.28 fcntl64 F
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
@@ -2139,3 +2139,4 @@ GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
GLIBC_2.27 wcstof64x F
GLIBC_2.27 wcstof64x_l F
+GLIBC_2.28 fcntl64 F