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

[Florian Weimer <fw at deneb dot enyo dot de>]

* Adhemerval Zanella:

>> I'm also a bit worried about this:
>> 
>> +      case F_GETLK64:
>> +      case F_OFD_GETLK:
>> +    {
>> +      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
>> +      };
>> 
>> Should we really perform translation for F_GETLK64?  That looks like a bug.
>
> My understanding is Linux expects a 'struct flock64' for both
> F_OFD_GETLK and F_GETLK64 (fs/fcntl.c:493) and with the patch fcntl
> on non-LFS mode will only be called with 'struct fcntl'.  So we need
> to transform to avoid the same issue as for OFD locks.

My concern is that existing code uses F_GETLK64 as a replacement for
the missing fcntl64 in non-LFS mode.  Here's some example in the criu
test suite:

| static int lock_reg(int fd, int cmd, int type, int whence,
| 		off_t offset, off_t len)
| {
| 	struct flock64 lock;
| 
| 	lock.l_type   = type;     /* F_RDLCK, F_WRLCK, F_UNLCK */
| 	lock.l_whence = whence;   /* SEEK_SET, SEEK_CUR, SEEK_END */
| 	lock.l_start  = offset;   /* byte offset, relative to l_whence */
| 	lock.l_len    = len;      /* #bytes (0 means to EOF) */
| 
| 	errno = 0;
| 	return fcntl(fd, cmd, &lock);
| }
| 
| #define set_read_lock(fd, whence, offset, len) \
| 	lock_reg(fd, F_SETLK64, F_RDLCK, whence, offset, len)
| #define set_write_lock(fd, whence, offset, len) \
| 	lock_reg(fd, F_SETLK64, F_WRLCK, whence, offset, len)

Doesn't this break (silently) after your changes?

I'm still not sure if translation is the right approach here.  Maybe
we can add warnings or compile-time failures on type mis-matches
instead.  Or provide non-variadic functions like fcntl_getlk64,
fcntl_ofd_getlk, fcntl_getfl etc.