Re: fcntl64 in linux32 emul

[christos%astron.com@localhost (Christos Zoulas)]

In article <47A0B793.1060308%pp2.inet.fi@localhost>,
Arto Huusko  <arto.huusko%pp2.inet.fi@localhost> wrote:
>-=-=-=-=-=-
>
>This post is about fcntl64 in linux32 emul, but the same issues may
>affect other system calls that have 64 bit variants as well.
>
>The first problem is that fcntl64 emul in compat/linux/common/linux_file64.c
>handles only F_GETLK64, F_SETLK64 and F_SETLKW64, and for other fcntl commands
>it calls linux_sys_fcntl(). But linux_sys_fcntl() is part of the *64-bit*
>linux emulation. This means that F_SETLK request from 32-bit linux binary
>is handled as if it came from 64-bit binary, and this means that the
>struct flock userland gave (32-bit off_t fields) and the struct flock the
>kernel expects (64-bit off_t fields) do not match at all.
>
>Looking at ktrace, you'll see something like this:
>
>    153      1 skype    CALL  fcntl64(3,6,0xfddfe7e4)
>    153      1 skype    RET   fcntl64 -1 errno -22 Invalid argument
>
>
>I did not check for sure, but I guess that even {G,S}ETLK64 variant
>is handled wrong, because struct flock64 looks like this:
>
>struct linux_flock64 {
>     short       l_type;
>     short       l_whence;
>     off_t       l_start;
>     off_t       l_len;
>     linux_pid_t l_pid;
>};
>
>When kernel is compiled on amd64, fields are aligned accodring to
>amd64 rules. What I did not check, but do suspect is that when
>compiled on 32 bit linux (with off_t fields loff_t in linux, which
>is long long), l_start and l_len are aligned on 4 byte boundary,
>not 8 as on amd64. Based on printfs I added to kernel, SETLK64
>requests look more sane when linux_flock64 structure is packed
>in NetBSD kernel (i.e. align l_start on 4 byte boundary).
>
>
>This all is from trying to get skype running on NetBSD/amd64 under
>linux32 emul. The program starts, but when trying to log in, it
>keeps on spinning and spinning, and does not log in.
>
>With the attached diff, I got skype started. The diff handles only
>SETLK and GETLK calls, but I suspect other commands and syscalls
>may need some treatment as well.
>
>-=-=-=-=-=-
>
>Index: linux_fcntl.h
>===================================================================
>RCS file: /cvsroot/src/sys/compat/linux/common/linux_fcntl.h,v
>retrieving revision 1.11
>diff -r1.11 linux_fcntl.h
>55a56,59
>> #ifdef       COMPAT_LINUX32
>>      linux32_off_t l_start;
>>      linux32_off_t l_len;
>> #else
>57a62
>> #endif
>61c66
>< struct linux_flock64 {
>---
>> struct __attribute__ ((__packed__)) linux_flock64 {
>Index: linux_file64.c
>===================================================================
>RCS file: /cvsroot/src/sys/compat/linux/common/linux_file64.c,v
>retrieving revision 1.44
>diff -r1.44 linux_file64.c
>264a265,306
>> static void
>> bsd_to_linux_flock(struct flock *bfp, struct linux_flock *lfp)
>> {
>> 
>>      lfp->l_start = bfp->l_start;
>>      lfp->l_len = bfp->l_len;
>>      lfp->l_pid = bfp->l_pid;
>>      lfp->l_whence = bfp->l_whence;
>>      switch (bfp->l_type) {
>>      case F_RDLCK:
>>              lfp->l_type = LINUX_F_RDLCK;
>>              break;
>>      case F_UNLCK:
>>              lfp->l_type = LINUX_F_UNLCK;
>>              break;
>>      case F_WRLCK:
>>              lfp->l_type = LINUX_F_WRLCK;
>>              break;
>>      }
>> }
>> 
>> static void
>> linux_to_bsd_flock(struct linux_flock *lfp, struct flock *bfp)
>> {
>> 
>>      bfp->l_start = lfp->l_start;
>>      bfp->l_len = lfp->l_len;
>>      bfp->l_pid = lfp->l_pid;
>>      bfp->l_whence = lfp->l_whence;
>>      switch (lfp->l_type) {
>>      case LINUX_F_RDLCK:
>>              bfp->l_type = F_RDLCK;
>>              break;
>>      case LINUX_F_UNLCK:
>>              bfp->l_type = F_UNLCK;
>>              break;
>>      case LINUX_F_WRLCK:
>>              bfp->l_type = F_WRLCK;
>>              break;
>>      }
>> }
>> 
>273a316
>>      struct linux_flock lfl32;
>296a340,357
>>      case LINUX_F_GETLK:
>>              if ((error = copyin(arg, &lfl32, sizeof lfl32)))
>>                      return error;
>>              linux_to_bsd_flock(&lfl32, &bfl);
>>              error = do_fcntl_lock(l, fd, F_GETLK, &bfl);
>>              if (error)
>>                      return error;
>>              bsd_to_linux_flock(&bfl, &lfl32);
>>              return copyout(&lfl32, arg, sizeof lfl32);
>> 
>>      case LINUX_F_SETLK:
>>      case LINUX_F_SETLKW:
>>              cmd = (cmd == LINUX_F_SETLK ? F_SETLK : F_SETLKW);
>>              if ((error = copyin(arg, &lfl32, sizeof lfl32))) {
>>                      return error;
>>              }
>>              linux_to_bsd_flock(&lfl32, &bfl);
>>              return do_fcntl_lock(l, fd, cmd, &bfl);

Can you send-pr this with diff -u?

christos