This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: Fix Linux fcntl OFD locks for non-LFS architectures (BZ#20251)
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: Samuel Thibault <samuel dot thibault at gnu dot org>
- Cc: libc-alpha at sourceware dot org
- Date: Thu, 15 Nov 2018 11:43:26 -0800
- Subject: Re: Fix Linux fcntl OFD locks for non-LFS architectures (BZ#20251)
- References: <20181115012457.ssscem7zfceifykq@function> <20181115012939.il77yr2gxkxyswu6@function> <20181115013648.xcxewlolirj35o3d@function> <901f8a33-88d5-73f4-90f4-ea0c1ee9b3a5@linaro.org> <20181115185350.umini454u67gtbco@function>
On 15/11/2018 10:53, Samuel Thibault wrote:
> Adhemerval Zanella, le jeu. 15 nov. 2018 09:45:15 -0800, a ecrit:
>> On 14/11/2018 17:36, Samuel Thibault wrote:
>>> Samuel Thibault, le jeu. 15 nov. 2018 02:29:39 +0100, a ecrit:
>>>> Samuel Thibault, le jeu. 15 nov. 2018 02:24:57 +0100, a ecrit:
>>>>> In login/utmp_file.c you have replaced calling __fcntl_nocancel by
>>>>> __fcntl64_nocancel, but shouldn't struct flock be replaced by struct
>>>>> flock64 too?
>>>>
>>>> Ah, no, applications *have* to use F_SETLK64 to use struct flock64, is
>>>> that it?
>>>
>>> Mmm, no, as I read the Linux implementation, when calling fcntl(), one
>>> has to use F_SETLK64 to be able to use struct flock64, but when calling
>>> fcntl64(), struct flock64 is always used,a and thus login/utmp_file.c
>>> should really be useing struct flock64?
>>
>> At least for Linux this specific usage is supported. For 32-bit architectures,
>> Linux fcntl64 does:
>>
>> 471 #if BITS_PER_LONG == 32
>> 472 SYSCALL_DEFINE3(fcntl64, unsigned int, fd, unsigned int, cmd,
>> 473 unsigned long, arg)
>> 474 {
>> [...]
>> 492 switch (cmd) {
>> [...]
>> 511 default:
>> 512 err = do_fcntl(fd, cmd, arg, f.file);
>> 513 break;
>> 514 }
>
> Where is this code? I don't find it in the glibc repository, neither on
> master nor on release/2.28/master.
This is from Linux kernel, file fs/fcntl.c.
>
>> The issue that BZ#20251 is using this same scenario but with
>> Linux-specific OFD locks (which is not supported).
>>
>> However I agree that this is confusing and I think it would be an
>> improvement if we explicit avoid use non-LFS interfaces within glibc.
>
> It doesn't seem only confusing to me, but actually bogus. AIUI, on i386
> either:
>
> - __USE_FILE_OFFSET64 is defined, struct flock is made 64bits, F_*LK are
> #defined to the F_*LK64 64bit variants, fcntl() thus uses 64bit values.
>
> - __USE_LARGEFILE64 is defined, struct flock is kept 32bits, struct
> flock64 is available as 64bit variant which can be used either with
> fcntl() with F_*LK64 variants, or with fcntl64.
>
> - none of the above is defined, struct flock is kept 32bits, flock64 is
> not available.
>
> AFAICT (just tested now), login/utmp_file.c is built with
> __USE_LARGEFILE64, not __USE_FILE_OFFSET64, and thus passing a struct
> flock (thus 32bit) to fcntl64 is bogus, since fcntl64 will consider it
> 64bits. I made sure with a
>
> _Static_assert(sizeof(struct flock) == sizeof(struct flock64), "oops");
>
> inside the LOCK_FILE macro itself, and it fails on i386, so I'm really
> tempted to make the fl there an flock64 not to fix confusion, but to
> actually fix a bug.
>
> Samuel
The scenario of using F_{GET,SET}LK for architectures with different types
for off_t and off64_t is, at least for Linux, to support both non-LFS
and LFS variant with fcntl64. The size of the advisory lock will deducted
from F_SETLKW or F_SETLKW64 and for Linux this is handled transparently
by the kernel.
The BZ#20251 issue was in fact this assumption is not true for OFD locks,
where it does not provide the F_OFD_*64 variant and thus fcntl64 you must
use a lock64.
But I do not oppose to use just LFS API explicit internally on glibc,
in this case one need to not only change the flock to flock64 but also
use F_SETLKW64.