[PATCH v2 0/3] linux-user: Fix fcntl64() and accept4() for 32-bit targets

[PATCH v2 0/3] linux-user: Fix fcntl64() and accept4() for 32-bit targets

[Helge Deller]

A set of 3 patches:
The first two patches fix fcntl64() and accept4().
the 3rd patch enhances the strace output for pread64/pwrite64().

I dropped my mmap2 patch in favour of Richard's proposal:
https://patchew.org/QEMU/20230630132159.376995-1-richard.henderson@linaro.org/20230630132159.376995-12-richard.henderson@linaro.org/

Changes:
v2:
- rephrased commmit logs
- return O_LARGFILE for fcntl() syscall too
- dropped #ifdefs in accept4() patch
- Dropped my mmap2() patch (former patch #3)
- added r-b from Richard to 3rd patch

Helge

Helge Deller (3):
  linux-user: Fix fcntl() and fcntl64() to return O_LARGEFILE for 32-bit
    targets
  linux-user: Fix accept4(SOCK_NONBLOCK) syscall
  linux-user: Improve strace output of pread64() and pwrite64()

 linux-user/strace.c    | 19 +++++++++++++++++++
 linux-user/strace.list |  4 ++--
 linux-user/syscall.c   | 16 +++++++++++++++-
 3 files changed, 36 insertions(+), 3 deletions(-)

--
2.41.0

[PATCH v2 1/3] linux-user: Fix fcntl() and fcntl64() to return O_LARGEFILE for 32-bit targets

[Helge Deller]

When running a 32-bit guest on a 64-bit host, fcntl[64](F_GETFL) should
return with the TARGET_O_LARGEFILE flag set, because all 64-bit hosts
support large files unconditionally.

But on 64-bit hosts, O_LARGEFILE has the value 0, so the flag
translation can't be done with the fcntl_flags_tbl[]. Instead add the
TARGET_O_LARGEFILE flag afterwards.

Note that for 64-bit guests the compiler will optimize away this code,
since TARGET_O_LARGEFILE is zero.

Signed-off-by: Helge Deller <deller@gmx.de>
---
 linux-user/syscall.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 08162cc966..10f05b1e55 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7132,6 +7132,10 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
         ret = get_errno(safe_fcntl(fd, host_cmd, arg));
         if (ret >= 0) {
             ret = host_to_target_bitmask(ret, fcntl_flags_tbl);
+            /* tell 32-bit guests it uses largefile on 64-bit hosts: */
+            if (O_LARGEFILE == 0 && HOST_LONG_BITS == 64) {
+                ret |= TARGET_O_LARGEFILE;
+            }
         }
         break;

--
2.41.0

Re: [PATCH v2 1/3] linux-user: Fix fcntl() and fcntl64() to return O_LARGEFILE for 32-bit targets

[Richard Henderson]

On 7/8/23 06:42, Helge Deller wrote:
> When running a 32-bit guest on a 64-bit host, fcntl[64](F_GETFL) should
> return with the TARGET_O_LARGEFILE flag set, because all 64-bit hosts
> support large files unconditionally.
> 
> But on 64-bit hosts, O_LARGEFILE has the value 0, so the flag
> translation can't be done with the fcntl_flags_tbl[]. Instead add the
> TARGET_O_LARGEFILE flag afterwards.
> 
> Note that for 64-bit guests the compiler will optimize away this code,
> since TARGET_O_LARGEFILE is zero.
> 
> Signed-off-by: Helge Deller<deller@gmx.de>
> ---
>   linux-user/syscall.c | 4 ++++
>   1 file changed, 4 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH v2 2/3] linux-user: Fix accept4(SOCK_NONBLOCK) syscall

[Helge Deller]

The Linux accept4() syscall allows two flags only: SOCK_NONBLOCK and
SOCK_CLOEXEC, and returns -EINVAL if any other bits have been set.

Change the qemu implementation accordingly, which means we can not use
the fcntl_flags_tbl[] translation table which allows too many other
values.

Beside the correction in behaviour, this actually fixes the accept4()
emulation for hppa, mips and alpha targets for which SOCK_NONBLOCK is
different than TARGET_SOCK_NONBLOCK (aka O_NONBLOCK).

The fix can be verified with the testcase of the debian lwt package,
which hangs forever in a read() syscall without this patch.

Signed-off-by: Helge Deller <deller@gmx.de>
---
 linux-user/syscall.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 10f05b1e55..9b9e3bd5e3 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3440,7 +3440,17 @@ static abi_long do_accept4(int fd, abi_ulong target_addr,
     abi_long ret;
     int host_flags;

-    host_flags = target_to_host_bitmask(flags, fcntl_flags_tbl);
+    if (flags & ~(TARGET_SOCK_CLOEXEC | TARGET_SOCK_NONBLOCK)) {
+        return -TARGET_EINVAL;
+    }
+
+    host_flags = 0;
+    if (flags & TARGET_SOCK_NONBLOCK) {
+        host_flags |= SOCK_NONBLOCK;
+    }
+    if (flags & TARGET_SOCK_CLOEXEC) {
+        host_flags |= SOCK_CLOEXEC;
+    }

     if (target_addr == 0) {
         return get_errno(safe_accept4(fd, NULL, NULL, host_flags));
--
2.41.0

Re: [PATCH v2 2/3] linux-user: Fix accept4(SOCK_NONBLOCK) syscall

[Richard Henderson]

On 7/8/23 06:42, Helge Deller wrote:
> The Linux accept4() syscall allows two flags only: SOCK_NONBLOCK and
> SOCK_CLOEXEC, and returns -EINVAL if any other bits have been set.
> 
> Change the qemu implementation accordingly, which means we can not use
> the fcntl_flags_tbl[] translation table which allows too many other
> values.
> 
> Beside the correction in behaviour, this actually fixes the accept4()
> emulation for hppa, mips and alpha targets for which SOCK_NONBLOCK is
> different than TARGET_SOCK_NONBLOCK (aka O_NONBLOCK).
> 
> The fix can be verified with the testcase of the debian lwt package,
> which hangs forever in a read() syscall without this patch.
> 
> Signed-off-by: Helge Deller<deller@gmx.de>
> ---
>   linux-user/syscall.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v2 2/3] linux-user: Fix accept4(SOCK_NONBLOCK) syscall

[Michael Tokarev]

08.07.2023 08:42, Helge Deller wrote:
> The Linux accept4() syscall allows two flags only: SOCK_NONBLOCK and
> SOCK_CLOEXEC, and returns -EINVAL if any other bits have been set.
> 
> Change the qemu implementation accordingly, which means we can not use
> the fcntl_flags_tbl[] translation table which allows too many other
> values.
> 
> Beside the correction in behaviour, this actually fixes the accept4()
> emulation for hppa, mips and alpha targets for which SOCK_NONBLOCK is
> different than TARGET_SOCK_NONBLOCK (aka O_NONBLOCK).
> 
> The fix can be verified with the testcase of the debian lwt package,
> which hangs forever in a read() syscall without this patch.

This smells like -stable material too.  Queued this one.

Thanks,

/mjt

[PATCH v2 3/3] linux-user: Improve strace output of pread64() and pwrite64()

[Helge Deller]

Make the strace look nicer for those two syscalls.

Signed-off-by: Helge Deller <deller@gmx.de>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/strace.c    | 19 +++++++++++++++++++
 linux-user/strace.list |  4 ++--
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index aad2b62ca4..669200c4a4 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -3999,6 +3999,25 @@ print_tgkill(CPUArchState *cpu_env, const struct syscallname *name,
 }
 #endif

+#if defined(TARGET_NR_pread64) || defined(TARGET_NR_pwrite64)
+static void
+print_pread64(CPUArchState *cpu_env, const struct syscallname *name,
+        abi_long arg0, abi_long arg1, abi_long arg2,
+        abi_long arg3, abi_long arg4, abi_long arg5)
+{
+    if (regpairs_aligned(cpu_env, TARGET_NR_pread64)) {
+        arg3 = arg4;
+        arg4 = arg5;
+    }
+    print_syscall_prologue(name);
+    print_raw_param("%d", arg0, 0);
+    print_pointer(arg1, 0);
+    print_raw_param("%d", arg2, 0);
+    print_raw_param("%" PRIu64, target_offset64(arg3, arg4), 1);
+    print_syscall_epilogue(name);
+}
+#endif
+
 #ifdef TARGET_NR_statx
 static void
 print_statx(CPUArchState *cpu_env, const struct syscallname *name,
diff --git a/linux-user/strace.list b/linux-user/strace.list
index c7808ea118..6655d4f26d 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -1068,7 +1068,7 @@
 { TARGET_NR_prctl, "prctl" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_pread64
-{ TARGET_NR_pread64, "pread64" , NULL, NULL, NULL },
+{ TARGET_NR_pread64, "pread64" , NULL, print_pread64, NULL },
 #endif
 #ifdef TARGET_NR_preadv
 { TARGET_NR_preadv, "preadv" , NULL, NULL, NULL },
@@ -1099,7 +1099,7 @@
 { TARGET_NR_putpmsg, "putpmsg" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_pwrite64
-{ TARGET_NR_pwrite64, "pwrite64" , NULL, NULL, NULL },
+{ TARGET_NR_pwrite64, "pwrite64" , NULL, print_pread64, NULL },
 #endif
 #ifdef TARGET_NR_pwritev
 { TARGET_NR_pwritev, "pwritev" , NULL, NULL, NULL },
--
2.41.0