Re: [Qemu-devel] [PATCH] fix fcntl support in linux-user.

[Arnaud Patard (Rtp) <arnaud.patard@rtp-net.org>]

[-- Attachment #1: Type: text/plain, Size: 977 bytes --]

Riku Voipio <riku.voipio@iki.fi> writes:

Hi,

> On Sun, Apr 19, 2009 at 10:45:39PM +0200, Arnaud Patard wrote:
>> 
>> The current code in do_fcntl is passing the target command as a host
>> command. This is introducing 2 problems:
>> 
>> - When building with "-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE", we
>>   may end up passing commands like F_GETFL instead of F_GETFL64. This is
>>   likely to return an error.
>> 
>> - The F_*64 constants doesn't always have the same value between host
>>   and target.
>
> This appears to be the case for some other fcntl constants as well. Howabout
> implementing this as a target_to_host_fcntl_cmd() mapping function instead?

Ok, done. New patch attached. Please comment :)

>
>> Without this patch locking is not working (My test-case was pwck with arm as
>> target and mips as host).
>
> Could you also add the MIPS target definition for F_*64 so that mips on arm
> would get fixed too?

oops. Forgot to include that in my patch


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fix_fcntl-2.patch --]
[-- Type: text/x-diff, Size: 5432 bytes --]

Index: qemu/linux-user/syscall.c
===================================================================
--- qemu.orig/linux-user/syscall.c
+++ qemu/linux-user/syscall.c
@@ -253,6 +253,18 @@ static bitmask_transtbl fcntl_flags_tbl[
   { 0, 0, 0, 0 }
 };
 
+static char target_to_host_fcntl_cmd[] = {
+  [ TARGET_F_GETLK    ] = F_GETLK,
+  [ TARGET_F_SETLK    ] = F_SETLK,
+  [ TARGET_F_SETLKW   ] = F_SETLKW,
+  [ TARGET_F_GETLK64  ] = F_GETLK64,
+  [ TARGET_F_SETLK64  ] = F_SETLK64,
+  [ TARGET_F_SETLKW64 ] = F_SETLKW64,
+  [ TARGET_F_SETOWN   ] = F_SETOWN,
+  [ TARGET_F_GETOWN   ] = F_GETOWN,
+  [ TARGET_F_SETSIG   ] = F_SETSIG,
+  [ TARGET_F_GETSIG   ] = F_GETSIG,
+};
 #define COPY_UTSNAME_FIELD(dest, src) \
   do { \
       /* __NEW_UTS_LEN doesn't include terminating null */ \
@@ -3365,6 +3377,7 @@ static abi_long do_fcntl(int fd, int cmd
     struct flock64 fl64;
     struct target_flock64 *target_fl64;
     abi_long ret;
+    int cmd2 = target_to_host_fcntl_cmd[cmd];
 
     switch(cmd) {
     case TARGET_F_GETLK:
@@ -3376,7 +3389,7 @@ static abi_long do_fcntl(int fd, int cmd
         fl.l_len = tswapl(target_fl->l_len);
         fl.l_pid = tswapl(target_fl->l_pid);
         unlock_user_struct(target_fl, arg, 0);
-        ret = get_errno(fcntl(fd, cmd, &fl));
+        ret = get_errno(fcntl(fd, cmd2, &fl));
         if (ret == 0) {
             if (!lock_user_struct(VERIFY_WRITE, target_fl, arg, 0))
                 return -TARGET_EFAULT;
@@ -3399,7 +3412,7 @@ static abi_long do_fcntl(int fd, int cmd
         fl.l_len = tswapl(target_fl->l_len);
         fl.l_pid = tswapl(target_fl->l_pid);
         unlock_user_struct(target_fl, arg, 0);
-        ret = get_errno(fcntl(fd, cmd, &fl));
+        ret = get_errno(fcntl(fd, cmd2, &fl));
         break;
 
     case TARGET_F_GETLK64:
@@ -3411,7 +3424,7 @@ static abi_long do_fcntl(int fd, int cmd
         fl64.l_len = tswapl(target_fl64->l_len);
         fl64.l_pid = tswap16(target_fl64->l_pid);
         unlock_user_struct(target_fl64, arg, 0);
-        ret = get_errno(fcntl(fd, cmd >> 1, &fl64));
+        ret = get_errno(fcntl(fd, cmd2, &fl64));
         if (ret == 0) {
             if (!lock_user_struct(VERIFY_WRITE, target_fl64, arg, 0))
                 return -TARGET_EFAULT;
@@ -3433,18 +3446,25 @@ static abi_long do_fcntl(int fd, int cmd
         fl64.l_len = tswapl(target_fl64->l_len);
         fl64.l_pid = tswap16(target_fl64->l_pid);
         unlock_user_struct(target_fl64, arg, 0);
-        ret = get_errno(fcntl(fd, cmd >> 1, &fl64));
+        ret = get_errno(fcntl(fd, cmd2, &fl64));
         break;
 
-    case F_GETFL:
-        ret = get_errno(fcntl(fd, cmd, arg));
+    case TARGET_F_GETFL:
+        ret = get_errno(fcntl(fd, cmd2, arg));
         if (ret >= 0) {
             ret = host_to_target_bitmask(ret, fcntl_flags_tbl);
         }
         break;
 
-    case F_SETFL:
-        ret = get_errno(fcntl(fd, cmd, target_to_host_bitmask(arg, fcntl_flags_tbl)));
+    case TARGET_F_SETFL:
+        ret = get_errno(fcntl(fd, cmd2, target_to_host_bitmask(arg, fcntl_flags_tbl)));
+        break;
+
+    case TARGET_F_SETOWN:
+    case TARGET_F_GETOWN:
+    case TARGET_F_SETSIG:
+    case TARGET_F_GETSIG:
+        ret = get_errno(fcntl(fd, cmd2, arg));
         break;
 
     default:
@@ -6222,20 +6242,7 @@ abi_long do_syscall(void *cpu_env, int n
 	struct target_eabi_flock64 *target_efl;
 #endif
 
-        switch(arg2){
-        case TARGET_F_GETLK64:
-            cmd = F_GETLK64;
-            break;
-        case TARGET_F_SETLK64:
-            cmd = F_SETLK64;
-            break;
-        case TARGET_F_SETLKW64:
-            cmd = F_SETLK64;
-            break;
-        default:
-            cmd = arg2;
-            break;
-        }
+	cmd = target_to_host_fcntl_cmd[arg2];
 
         switch(arg2) {
         case TARGET_F_GETLK64:
@@ -6315,7 +6322,7 @@ abi_long do_syscall(void *cpu_env, int n
             ret = get_errno(fcntl(arg1, cmd, &fl));
 	    break;
         default:
-            ret = do_fcntl(arg1, cmd, arg3);
+            ret = do_fcntl(arg1, arg2, arg3);
             break;
         }
 	break;
Index: qemu/linux-user/syscall_defs.h
===================================================================
--- qemu.orig/linux-user/syscall_defs.h
+++ qemu/linux-user/syscall_defs.h
@@ -1662,6 +1662,12 @@ struct target_statfs64 {
 #define TARGET_F_SETLKW        9
 #define TARGET_F_SETOWN        5       /*  for sockets. */
 #define TARGET_F_GETOWN        6       /*  for sockets. */
+#elif defined(TARGET_MIPS)
+#define TARGET_F_GETLK         14
+#define TARGET_F_SETLK         6
+#define TARGET_F_SETLKW        7
+#define TARGET_F_SETOWN        24       /*  for sockets. */
+#define TARGET_F_GETOWN        25       /*  for sockets. */
 #else
 #define TARGET_F_GETLK         5
 #define TARGET_F_SETLK         6
@@ -1673,10 +1679,15 @@ struct target_statfs64 {
 #define TARGET_F_SETSIG        10      /*  for sockets. */
 #define TARGET_F_GETSIG        11      /*  for sockets. */
 
+#if defined(TARGET_MIPS)
+#define TARGET_F_GETLK64       33      /*  using 'struct flock64' */
+#define TARGET_F_SETLK64       34
+#define TARGET_F_SETLKW64      35
+#else
 #define TARGET_F_GETLK64       12      /*  using 'struct flock64' */
 #define TARGET_F_SETLK64       13
 #define TARGET_F_SETLKW64      14
-
+#endif
 #if defined (TARGET_ARM)
 #define TARGET_O_ACCMODE          0003
 #define TARGET_O_RDONLY             00