Re: Fix Linux fcntl OFD locks for non-LFS architectures (BZ#20251)

[Samuel Thibault <samuel dot thibault at gnu dot org>]

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.

> 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