qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL 0/7] Linux user for 5.1 patches
@ 2020-07-14  7:32 Laurent Vivier
  2020-07-14  7:32 ` [PULL 1/7] linux-user: Use EPROTONOSUPPORT for unimplemented netlink protocols Laurent Vivier
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Laurent Vivier @ 2020-07-14  7:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier

The following changes since commit 5c65b1f135ff09d24827fa3a17e56a4f8a032cd5:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200713' into staging (2020-07-13 15:14:48 +0100)

are available in the Git repository at:

  git://github.com/vivier/qemu.git tags/linux-user-for-5.1-pull-request

for you to fetch changes up to 42b16184d016d48d167229a1ddb89b3671c77440:

  linux-user: fix print_syscall_err() when syscall returned value is negative (2020-07-14 09:29:14 +0200)

----------------------------------------------------------------
linux-user branch 20200714

Fix strace errno management
Fix Coverity erros in ioctl straces
Fix some netlinks errors
Fix semtimedop

----------------------------------------------------------------

Josh Kunz (1):
  linux-user: Use EPROTONOSUPPORT for unimplemented netlink protocols

Laurent Vivier (5):
  linux-user: Fix Coverity CID 1430271 / CID 1430272
  linux-user: add new netlink types
  linux-user: add netlink RTM_SETLINK command
  linux-user: fix the errno value in print_syscall_err()
  linux-user: fix print_syscall_err() when syscall returned value is
    negative

Matus Kysel (1):
  linux-user: refactor ipc syscall and support of semtimedop syscall

 linux-user/fd-trans.c |  5 +++
 linux-user/strace.c   | 52 +++++++++++++-------------
 linux-user/syscall.c  | 86 +++++++++++++++++++++++++++++++++++++++----
 3 files changed, 108 insertions(+), 35 deletions(-)

-- 
2.26.2



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PULL 1/7] linux-user: Use EPROTONOSUPPORT for unimplemented netlink protocols
  2020-07-14  7:32 [PULL 0/7] Linux user for 5.1 patches Laurent Vivier
@ 2020-07-14  7:32 ` Laurent Vivier
  2020-07-14  7:32 ` [PULL 2/7] linux-user: refactor ipc syscall and support of semtimedop syscall Laurent Vivier
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Laurent Vivier @ 2020-07-14  7:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Josh Kunz, Laurent Vivier

From: Josh Kunz <jkz@google.com>

Linux uses the EPROTONOSUPPORT error code[1] if the users requests a
netlink socket with an unsupported netlink protocol. This change
switches linux-user to use the same code as Linux, instead of
EPFNOSUPPORT (which AFAIK is just an anachronistic version of
EAFNOSUPPORT).

Tested by compiling all linux-user targets on x86.

[1]:
https://github.com/torvalds/linux/blob/bfe91da29bfad9941d5d703d45e29f0812a20724/net/netlink/af_netlink.c#L683

Signed-off-by: Josh Kunz <jkz@google.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20200707001036.1671982-1-jkz@google.com>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/syscall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 98ea86ca81fb..e9f53340cd65 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -2990,7 +2990,7 @@ static abi_long do_socket(int domain, int type, int protocol)
 #endif
          protocol == NETLINK_KOBJECT_UEVENT ||
          protocol == NETLINK_AUDIT)) {
-        return -TARGET_EPFNOSUPPORT;
+        return -TARGET_EPROTONOSUPPORT;
     }
 
     if (domain == AF_PACKET ||
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PULL 2/7] linux-user: refactor ipc syscall and support of semtimedop syscall
  2020-07-14  7:32 [PULL 0/7] Linux user for 5.1 patches Laurent Vivier
  2020-07-14  7:32 ` [PULL 1/7] linux-user: Use EPROTONOSUPPORT for unimplemented netlink protocols Laurent Vivier
@ 2020-07-14  7:32 ` Laurent Vivier
  2020-07-14  7:32 ` [PULL 3/7] linux-user: Fix Coverity CID 1430271 / CID 1430272 Laurent Vivier
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Laurent Vivier @ 2020-07-14  7:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier, Matus Kysel

From: Matus Kysel <mkysel@tachyum.com>

Refactoring ipc syscall for s390x and SPARC, so it matches glibc implementation

We should add support of semtimedop syscall as new version of glibc
2.31 uses semop based on semtimedop
(commit: https://gitlab.com/freedesktop-sdk/mirrors/sourceware/glibc/-/commit/765cdd0bffd77960ae852104fc4ea5edcdb8aed3 ).

Signed-off-by: Matus Kysel <mkysel@tachyum.com>
Message-Id: <20200626124612.58593-2-mkysel@tachyum.com>
Message-Id: <20200626124612.58593-3-mkysel@tachyum.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
[lv: merged PATCH 1 & 2 to avoid build break on PATCH 1]
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/syscall.c | 84 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 77 insertions(+), 7 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index e9f53340cd65..1211e759c26c 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -817,9 +817,14 @@ safe_syscall4(int, clock_nanosleep, const clockid_t, clock, int, flags,
               const struct timespec *, req, struct timespec *, rem)
 #endif
 #ifdef __NR_ipc
+#ifdef __s390x__
+safe_syscall5(int, ipc, int, call, long, first, long, second, long, third,
+              void *, ptr)
+#else
 safe_syscall6(int, ipc, int, call, long, first, long, second, long, third,
               void *, ptr, long, fifth)
 #endif
+#endif
 #ifdef __NR_msgsnd
 safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
               int, flags)
@@ -1230,7 +1235,8 @@ static inline abi_long copy_to_user_timeval64(abi_ulong target_tv_addr,
     defined(TARGET_NR_pselect6) || defined(TARGET_NR_pselect6) || \
     defined(TARGET_NR_nanosleep) || defined(TARGET_NR_clock_settime) || \
     defined(TARGET_NR_utimensat) || defined(TARGET_NR_mq_timedsend) || \
-    defined(TARGET_NR_mq_timedreceive)
+    defined(TARGET_NR_mq_timedreceive) || defined(TARGET_NR_ipc) || \
+    defined(TARGET_NR_semop) || defined(TARGET_NR_semtimedop)
 static inline abi_long target_to_host_timespec(struct timespec *host_ts,
                                                abi_ulong target_addr)
 {
@@ -3878,25 +3884,53 @@ static inline abi_long target_to_host_sembuf(struct sembuf *host_sembuf,
     return 0;
 }
 
-static inline abi_long do_semop(int semid, abi_long ptr, unsigned nsops)
+#if defined(TARGET_NR_ipc) || defined(TARGET_NR_semop) || \
+    defined(TARGET_NR_semtimedop)
+
+/*
+ * This macro is required to handle the s390 variants, which passes the
+ * arguments in a different order than default.
+ */
+#ifdef __s390x__
+#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
+  (__nsops), (__timeout), (__sops)
+#else
+#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
+  (__nsops), 0, (__sops), (__timeout)
+#endif
+
+static inline abi_long do_semtimedop(int semid,
+                                     abi_long ptr,
+                                     unsigned nsops,
+                                     abi_long timeout)
 {
     struct sembuf sops[nsops];
+    struct timespec ts, *pts = NULL;
     abi_long ret;
 
+    if (timeout) {
+        pts = &ts;
+        if (target_to_host_timespec(pts, timeout)) {
+            return -TARGET_EFAULT;
+        }
+    }
+
     if (target_to_host_sembuf(sops, ptr, nsops))
         return -TARGET_EFAULT;
 
     ret = -TARGET_ENOSYS;
 #ifdef __NR_semtimedop
-    ret = get_errno(safe_semtimedop(semid, sops, nsops, NULL));
+    ret = get_errno(safe_semtimedop(semid, sops, nsops, pts));
 #endif
 #ifdef __NR_ipc
     if (ret == -TARGET_ENOSYS) {
-        ret = get_errno(safe_ipc(IPCOP_semtimedop, semid, nsops, 0, sops, 0));
+        ret = get_errno(safe_ipc(IPCOP_semtimedop, semid,
+                                 SEMTIMEDOP_IPC_ARGS(nsops, sops, (long)pts)));
     }
 #endif
     return ret;
 }
+#endif
 
 struct target_msqid_ds
 {
@@ -4056,8 +4090,13 @@ static inline abi_long do_msgsnd(int msqid, abi_long msgp,
 #endif
 #ifdef __NR_ipc
     if (ret == -TARGET_ENOSYS) {
+#ifdef __s390x__
+        ret = get_errno(safe_ipc(IPCOP_msgsnd, msqid, msgsz, msgflg,
+                                 host_mb));
+#else
         ret = get_errno(safe_ipc(IPCOP_msgsnd, msqid, msgsz, msgflg,
                                  host_mb, 0));
+#endif
     }
 #endif
     g_free(host_mb);
@@ -4066,6 +4105,20 @@ static inline abi_long do_msgsnd(int msqid, abi_long msgp,
     return ret;
 }
 
+#ifdef __NR_ipc
+#if defined(__sparc__)
+/* SPARC for msgrcv it does not use the kludge on final 2 arguments.  */
+#define MSGRCV_ARGS(__msgp, __msgtyp) __msgp, __msgtyp
+#elif defined(__s390x__)
+/* The s390 sys_ipc variant has only five parameters.  */
+#define MSGRCV_ARGS(__msgp, __msgtyp) \
+    ((long int[]){(long int)__msgp, __msgtyp})
+#else
+#define MSGRCV_ARGS(__msgp, __msgtyp) \
+    ((long int[]){(long int)__msgp, __msgtyp}), 0
+#endif
+#endif
+
 static inline abi_long do_msgrcv(int msqid, abi_long msgp,
                                  ssize_t msgsz, abi_long msgtyp,
                                  int msgflg)
@@ -4094,7 +4147,7 @@ static inline abi_long do_msgrcv(int msqid, abi_long msgp,
 #ifdef __NR_ipc
     if (ret == -TARGET_ENOSYS) {
         ret = get_errno(safe_ipc(IPCOP_CALL(1, IPCOP_msgrcv), msqid, msgsz,
-                        msgflg, host_mb, msgtyp));
+                        msgflg, MSGRCV_ARGS(host_mb, msgtyp)));
     }
 #endif
 
@@ -4372,7 +4425,20 @@ static abi_long do_ipc(CPUArchState *cpu_env,
 
     switch (call) {
     case IPCOP_semop:
-        ret = do_semop(first, ptr, second);
+        ret = do_semtimedop(first, ptr, second, 0);
+        break;
+    case IPCOP_semtimedop:
+    /*
+     * The s390 sys_ipc variant has only five parameters instead of six
+     * (as for default variant) and the only difference is the handling of
+     * SEMTIMEDOP where on s390 the third parameter is used as a pointer
+     * to a struct timespec where the generic variant uses fifth parameter.
+     */
+#if defined(TARGET_S390X)
+        ret = do_semtimedop(first, ptr, second, third);
+#else
+        ret = do_semtimedop(first, ptr, second, fifth);
+#endif
         break;
 
     case IPCOP_semget:
@@ -9684,7 +9750,11 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR_semop
     case TARGET_NR_semop:
-        return do_semop(arg1, arg2, arg3);
+        return do_semtimedop(arg1, arg2, arg3, 0);
+#endif
+#ifdef TARGET_NR_semtimedop
+    case TARGET_NR_semtimedop:
+        return do_semtimedop(arg1, arg2, arg3, arg4);
 #endif
 #ifdef TARGET_NR_semctl
     case TARGET_NR_semctl:
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PULL 3/7] linux-user: Fix Coverity CID 1430271 / CID 1430272
  2020-07-14  7:32 [PULL 0/7] Linux user for 5.1 patches Laurent Vivier
  2020-07-14  7:32 ` [PULL 1/7] linux-user: Use EPROTONOSUPPORT for unimplemented netlink protocols Laurent Vivier
  2020-07-14  7:32 ` [PULL 2/7] linux-user: refactor ipc syscall and support of semtimedop syscall Laurent Vivier
@ 2020-07-14  7:32 ` Laurent Vivier
  2020-07-14  7:32 ` [PULL 4/7] linux-user: add new netlink types Laurent Vivier
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Laurent Vivier @ 2020-07-14  7:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Laurent Vivier

In new functions print_ioctl() and print_syscall_ret_ioctl(), we don't
check if lock_user() returns NULL and this would cause a segfault in
thunk_print().

If lock_user() returns NULL don't call thunk_print() but prints only the
value of the (invalid) pointer.

Tested with:

    # cat ioctl.c
    #include <unistd.h>
    #include <sys/ioctl.h>

    int main(void)
    {
        int ret;

        ret = ioctl(STDOUT_FILENO, TCGETS, 0xdeadbeef);
        ret = ioctl(STDOUT_FILENO, TCSETSF, 0xdeadbeef);
        return 0;
    }
    # QEMU_STRACE= ./ioctl
    ...
    578 ioctl(1,TCGETS,0xdeadbeef) = -1 errno=2 (Bad address)
    578 ioctl(1,TCSETSF,0xdeadbeef) = -1 errno=2 (Bad address)
    ...
    # QEMU_STRACE= passwd
    ...
    623 ioctl(0,TCGETS,0x3fffed04) = 0 ({})
    623 ioctl(0,TCSETSF,{}) = 0
    ...

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Fixes: 79482e5987c8 ("linux-user: Add strace support for printing arguments of ioctl()")
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/strace.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 5235b2260cdd..39554d903911 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -889,8 +889,12 @@ print_syscall_ret_ioctl(const struct syscallname *name, abi_long ret,
             arg_type++;
             target_size = thunk_type_size(arg_type, 0);
             argptr = lock_user(VERIFY_READ, arg2, target_size, 1);
-            thunk_print(argptr, arg_type);
-            unlock_user(argptr, arg2, target_size);
+            if (argptr) {
+                thunk_print(argptr, arg_type);
+                unlock_user(argptr, arg2, target_size);
+            } else {
+                print_pointer(arg2, 1);
+            }
             qemu_log(")");
         }
     }
@@ -3119,8 +3123,12 @@ print_ioctl(const struct syscallname *name,
                     arg_type++;
                     target_size = thunk_type_size(arg_type, 0);
                     argptr = lock_user(VERIFY_READ, arg2, target_size, 1);
-                    thunk_print(argptr, arg_type);
-                    unlock_user(argptr, arg2, target_size);
+                    if (argptr) {
+                        thunk_print(argptr, arg_type);
+                        unlock_user(argptr, arg2, target_size);
+                    } else {
+                        print_pointer(arg2, 1);
+                    }
                     break;
                 }
                 break;
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PULL 4/7] linux-user: add new netlink types
  2020-07-14  7:32 [PULL 0/7] Linux user for 5.1 patches Laurent Vivier
                   ` (2 preceding siblings ...)
  2020-07-14  7:32 ` [PULL 3/7] linux-user: Fix Coverity CID 1430271 / CID 1430272 Laurent Vivier
@ 2020-07-14  7:32 ` Laurent Vivier
  2020-07-14  7:32 ` [PULL 5/7] linux-user: add netlink RTM_SETLINK command Laurent Vivier
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Laurent Vivier @ 2020-07-14  7:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier

Only implement IFLA_PERM_ADDRESS to fix the following error:

  Unknown host QEMU_IFLA type: 54

The couple of other ones, IFLA_PROP_LIST and IFLA_ALT_IFNAME, have
been introduced to be used with RTM_NEWLINKPROP, RTM_DELLINKPROP and
RTM_GETLINKPROP that are not implemented by QEMU.

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20200709072332.890440-1-laurent@vivier.eu>
---
 linux-user/fd-trans.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/linux-user/fd-trans.c b/linux-user/fd-trans.c
index c0687c52e62b..5d49a53552b2 100644
--- a/linux-user/fd-trans.c
+++ b/linux-user/fd-trans.c
@@ -133,6 +133,9 @@ enum {
     QEMU_IFLA_NEW_IFINDEX,
     QEMU_IFLA_MIN_MTU,
     QEMU_IFLA_MAX_MTU,
+    QEMU_IFLA_PROP_LIST,
+    QEMU_IFLA_ALT_IFNAME,
+    QEMU_IFLA_PERM_ADDRESS,
     QEMU___IFLA_MAX
 };
 
@@ -807,6 +810,7 @@ static abi_long host_to_target_data_link_rtattr(struct rtattr *rtattr)
     /* binary stream */
     case QEMU_IFLA_ADDRESS:
     case QEMU_IFLA_BROADCAST:
+    case QEMU_IFLA_PERM_ADDRESS:
     /* string */
     case QEMU_IFLA_IFNAME:
     case QEMU_IFLA_QDISC:
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PULL 5/7] linux-user: add netlink RTM_SETLINK command
  2020-07-14  7:32 [PULL 0/7] Linux user for 5.1 patches Laurent Vivier
                   ` (3 preceding siblings ...)
  2020-07-14  7:32 ` [PULL 4/7] linux-user: add new netlink types Laurent Vivier
@ 2020-07-14  7:32 ` Laurent Vivier
  2020-07-14  7:32 ` [PULL 6/7] linux-user: fix the errno value in print_syscall_err() Laurent Vivier
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Laurent Vivier @ 2020-07-14  7:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier

This command is needed to be able to boot systemd in a container.

  $ sudo systemd-nspawn -D /chroot/armhf/sid/ -b
  Spawning container sid on /chroot/armhf/sid.
  Press ^] three times within 1s to kill container.
  systemd 245.6-2 running in system mode.
  Detected virtualization systemd-nspawn.
  Detected architecture arm.

  Welcome to Debian GNU/Linux bullseye/sid!

  Set hostname to <virt-arm>.
  Failed to enqueue loopback interface start request: Operation not supported
  Caught <SEGV>, dumped core as pid 3.
  Exiting PID 1...
  Container sid failed with error code 255.

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20200709072332.890440-2-laurent@vivier.eu>
---
 linux-user/fd-trans.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/linux-user/fd-trans.c b/linux-user/fd-trans.c
index 5d49a53552b2..1486c81aaa27 100644
--- a/linux-user/fd-trans.c
+++ b/linux-user/fd-trans.c
@@ -1204,6 +1204,7 @@ static abi_long target_to_host_data_route(struct nlmsghdr *nlh)
         break;
     case RTM_NEWLINK:
     case RTM_DELLINK:
+    case RTM_SETLINK:
         if (nlh->nlmsg_len >= NLMSG_LENGTH(sizeof(*ifi))) {
             ifi = NLMSG_DATA(nlh);
             ifi->ifi_type = tswap16(ifi->ifi_type);
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PULL 6/7] linux-user: fix the errno value in print_syscall_err()
  2020-07-14  7:32 [PULL 0/7] Linux user for 5.1 patches Laurent Vivier
                   ` (4 preceding siblings ...)
  2020-07-14  7:32 ` [PULL 5/7] linux-user: add netlink RTM_SETLINK command Laurent Vivier
@ 2020-07-14  7:32 ` Laurent Vivier
  2020-07-14  7:32 ` [PULL 7/7] linux-user: fix print_syscall_err() when syscall returned value is negative Laurent Vivier
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Laurent Vivier @ 2020-07-14  7:32 UTC (permalink / raw)
  To: qemu-devel
  Cc: Richard Henderson, Laurent Vivier, Filip.Bozuta,
	Philippe Mathieu-Daudé

errno of the target is returned as a negative value by the syscall,
not in the host errno variable.

The emulation of the target syscall can return an error while the
host doesn't set an errno value. Target errnos and host errnos can
also differ in some cases.

Fixes: c84be71f6854 ("linux-user: Extend strace support to enable argument printing after syscall execution")
Cc: Filip.Bozuta@syrmia.com
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
Message-Id: <20200708152435.706070-2-laurent@vivier.eu>
---
 linux-user/strace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 39554d903911..7769f53bd5ed 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -731,7 +731,7 @@ print_syscall_err(abi_long ret)
 
     qemu_log(" = ");
     if (ret < 0) {
-        qemu_log("-1 errno=%d", errno);
+        qemu_log("-1 errno=%d", (int)-ret);
         errstr = target_strerror(-ret);
         if (errstr) {
             qemu_log(" (%s)", errstr);
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PULL 7/7] linux-user: fix print_syscall_err() when syscall returned value is negative
  2020-07-14  7:32 [PULL 0/7] Linux user for 5.1 patches Laurent Vivier
                   ` (5 preceding siblings ...)
  2020-07-14  7:32 ` [PULL 6/7] linux-user: fix the errno value in print_syscall_err() Laurent Vivier
@ 2020-07-14  7:32 ` Laurent Vivier
  2020-07-14  7:59 ` [PULL 0/7] Linux user for 5.1 patches no-reply
  2020-07-14 20:21 ` Peter Maydell
  8 siblings, 0 replies; 10+ messages in thread
From: Laurent Vivier @ 2020-07-14  7:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier, Filip.Bozuta

print_syscall_err() relies on the sign of the returned value to know
if it is an errno value or not.

But in some cases the returned value can have the most signicant bit
set without being an errno.

This patch restores previous behaviour that was also checking if
we can decode the errno to validate it.

This patch fixes this kind of problem (qemu-m68k):

  root@sid:/# QEMU_STRACE= ls
  3 brk(NULL) = -1 errno=21473607683 uname(0x407fff8a) = 0

to become:

  root@sid:/# QEMU_STRACE= ls
  3 brk(NULL) = 0x8001e000
  3 uname(0xffffdf8a) = 0

Fixes: c84be71f6854 ("linux-user: Extend strace support to enable argument printing after syscall execution")
Cc: Filip.Bozuta@syrmia.com
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20200708152435.706070-3-laurent@vivier.eu>
---
 linux-user/strace.c | 36 +++++++++++++-----------------------
 1 file changed, 13 insertions(+), 23 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 7769f53bd5ed..13981341b327 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -724,19 +724,20 @@ print_ipc(const struct syscallname *name,
  * Variants for the return value output function
  */
 
-static void
+static bool
 print_syscall_err(abi_long ret)
 {
-    const char *errstr = NULL;
+    const char *errstr;
 
     qemu_log(" = ");
     if (ret < 0) {
-        qemu_log("-1 errno=%d", (int)-ret);
         errstr = target_strerror(-ret);
         if (errstr) {
-            qemu_log(" (%s)", errstr);
+            qemu_log("-1 errno=%d (%s)", (int)-ret, errstr);
+            return true;
         }
     }
+    return false;
 }
 
 static void
@@ -744,11 +745,10 @@ print_syscall_ret_addr(const struct syscallname *name, abi_long ret,
                        abi_long arg0, abi_long arg1, abi_long arg2,
                        abi_long arg3, abi_long arg4, abi_long arg5)
 {
-    print_syscall_err(ret);
-
-    if (ret >= 0) {
-        qemu_log("0x" TARGET_ABI_FMT_lx "\n", ret);
+    if (!print_syscall_err(ret)) {
+        qemu_log("0x" TARGET_ABI_FMT_lx, ret);
     }
+    qemu_log("\n");
 }
 
 #if 0 /* currently unused */
@@ -765,9 +765,7 @@ print_syscall_ret_newselect(const struct syscallname *name, abi_long ret,
                             abi_long arg0, abi_long arg1, abi_long arg2,
                             abi_long arg3, abi_long arg4, abi_long arg5)
 {
-    print_syscall_err(ret);
-
-    if (ret >= 0) {
+    if (!print_syscall_err(ret)) {
         qemu_log(" = 0x" TARGET_ABI_FMT_lx " (", ret);
         print_fdset(arg0, arg1);
         qemu_log(",");
@@ -796,9 +794,7 @@ print_syscall_ret_adjtimex(const struct syscallname *name, abi_long ret,
                            abi_long arg0, abi_long arg1, abi_long arg2,
                            abi_long arg3, abi_long arg4, abi_long arg5)
 {
-    print_syscall_err(ret);
-
-    if (ret >= 0) {
+    if (!print_syscall_err(ret)) {
         qemu_log(TARGET_ABI_FMT_ld, ret);
         switch (ret) {
         case TARGET_TIME_OK:
@@ -833,9 +829,7 @@ print_syscall_ret_listxattr(const struct syscallname *name, abi_long ret,
                             abi_long arg0, abi_long arg1, abi_long arg2,
                             abi_long arg3, abi_long arg4, abi_long arg5)
 {
-    print_syscall_err(ret);
-
-    if (ret >= 0) {
+    if (!print_syscall_err(ret)) {
         qemu_log(TARGET_ABI_FMT_ld, ret);
         qemu_log(" (list = ");
         if (arg1 != 0) {
@@ -866,9 +860,7 @@ print_syscall_ret_ioctl(const struct syscallname *name, abi_long ret,
                         abi_long arg0, abi_long arg1, abi_long arg2,
                         abi_long arg3, abi_long arg4, abi_long arg5)
 {
-    print_syscall_err(ret);
-
-    if (ret >= 0) {
+    if (!print_syscall_err(ret)) {
         qemu_log(TARGET_ABI_FMT_ld, ret);
 
         const IOCTLEntry *ie;
@@ -3197,9 +3189,7 @@ print_syscall_ret(int num, abi_long ret,
                                   arg1, arg2, arg3,
                                   arg4, arg5, arg6);
             } else {
-                print_syscall_err(ret);
-
-                if (ret >= 0) {
+                if (!print_syscall_err(ret)) {
                     qemu_log(TARGET_ABI_FMT_ld, ret);
                 }
                 qemu_log("\n");
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PULL 0/7] Linux user for 5.1 patches
  2020-07-14  7:32 [PULL 0/7] Linux user for 5.1 patches Laurent Vivier
                   ` (6 preceding siblings ...)
  2020-07-14  7:32 ` [PULL 7/7] linux-user: fix print_syscall_err() when syscall returned value is negative Laurent Vivier
@ 2020-07-14  7:59 ` no-reply
  2020-07-14 20:21 ` Peter Maydell
  8 siblings, 0 replies; 10+ messages in thread
From: no-reply @ 2020-07-14  7:59 UTC (permalink / raw)
  To: laurent; +Cc: qemu-devel, laurent

Patchew URL: https://patchew.org/QEMU/20200714073259.1464675-1-laurent@vivier.eu/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20200714073259.1464675-1-laurent@vivier.eu
Subject: [PULL 0/7] Linux user for 5.1 patches

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
ffeb691 linux-user: fix print_syscall_err() when syscall returned value is negative
415b123 linux-user: fix the errno value in print_syscall_err()
75a05ad linux-user: add netlink RTM_SETLINK command
124e290 linux-user: add new netlink types
2fb1965 linux-user: Fix Coverity CID 1430271 / CID 1430272
178a436 linux-user: refactor ipc syscall and support of semtimedop syscall
7901fa7 linux-user: Use EPROTONOSUPPORT for unimplemented netlink protocols

=== OUTPUT BEGIN ===
1/7 Checking commit 7901fa78df84 (linux-user: Use EPROTONOSUPPORT for unimplemented netlink protocols)
2/7 Checking commit 178a436bfe2d (linux-user: refactor ipc syscall and support of semtimedop syscall)
WARNING: architecture specific defines should be avoided
#29: FILE: linux-user/syscall.c:820:
+#ifdef __s390x__

WARNING: architecture specific defines should be avoided
#62: FILE: linux-user/syscall.c:3894:
+#ifdef __s390x__

ERROR: Macros with complex values should be enclosed in parenthesis
#63: FILE: linux-user/syscall.c:3895:
+#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
+  (__nsops), (__timeout), (__sops)

ERROR: Macros with complex values should be enclosed in parenthesis
#66: FILE: linux-user/syscall.c:3898:
+#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
+  (__nsops), 0, (__sops), (__timeout)

WARNING: architecture specific defines should be avoided
#111: FILE: linux-user/syscall.c:4093:
+#ifdef __s390x__

WARNING: architecture specific defines should be avoided
#125: FILE: linux-user/syscall.c:4108:
+#ifdef __NR_ipc

WARNING: architecture specific defines should be avoided
#126: FILE: linux-user/syscall.c:4109:
+#if defined(__sparc__)

ERROR: Macros with complex values should be enclosed in parenthesis
#128: FILE: linux-user/syscall.c:4111:
+#define MSGRCV_ARGS(__msgp, __msgtyp) __msgp, __msgtyp

ERROR: Macros with complex values should be enclosed in parenthesis
#134: FILE: linux-user/syscall.c:4117:
+#define MSGRCV_ARGS(__msgp, __msgtyp) \
+    ((long int[]){(long int)__msgp, __msgtyp}), 0

total: 4 errors, 5 warnings, 153 lines checked

Patch 2/7 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/7 Checking commit 2fb19656ca9e (linux-user: Fix Coverity CID 1430271 / CID 1430272)
4/7 Checking commit 124e290931a2 (linux-user: add new netlink types)
5/7 Checking commit 75a05adf0c46 (linux-user: add netlink RTM_SETLINK command)
6/7 Checking commit 415b12395796 (linux-user: fix the errno value in print_syscall_err())
7/7 Checking commit ffeb69146ef6 (linux-user: fix print_syscall_err() when syscall returned value is negative)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200714073259.1464675-1-laurent@vivier.eu/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PULL 0/7] Linux user for 5.1 patches
  2020-07-14  7:32 [PULL 0/7] Linux user for 5.1 patches Laurent Vivier
                   ` (7 preceding siblings ...)
  2020-07-14  7:59 ` [PULL 0/7] Linux user for 5.1 patches no-reply
@ 2020-07-14 20:21 ` Peter Maydell
  8 siblings, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2020-07-14 20:21 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: QEMU Developers

On Tue, 14 Jul 2020 at 08:34, Laurent Vivier <laurent@vivier.eu> wrote:
>
> The following changes since commit 5c65b1f135ff09d24827fa3a17e56a4f8a032cd5:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200713' into staging (2020-07-13 15:14:48 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/vivier/qemu.git tags/linux-user-for-5.1-pull-request
>
> for you to fetch changes up to 42b16184d016d48d167229a1ddb89b3671c77440:
>
>   linux-user: fix print_syscall_err() when syscall returned value is negative (2020-07-14 09:29:14 +0200)
>
> ----------------------------------------------------------------
> linux-user branch 20200714
>
> Fix strace errno management
> Fix Coverity erros in ioctl straces
> Fix some netlinks errors
> Fix semtimedop
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-07-14 20:25 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-14  7:32 [PULL 0/7] Linux user for 5.1 patches Laurent Vivier
2020-07-14  7:32 ` [PULL 1/7] linux-user: Use EPROTONOSUPPORT for unimplemented netlink protocols Laurent Vivier
2020-07-14  7:32 ` [PULL 2/7] linux-user: refactor ipc syscall and support of semtimedop syscall Laurent Vivier
2020-07-14  7:32 ` [PULL 3/7] linux-user: Fix Coverity CID 1430271 / CID 1430272 Laurent Vivier
2020-07-14  7:32 ` [PULL 4/7] linux-user: add new netlink types Laurent Vivier
2020-07-14  7:32 ` [PULL 5/7] linux-user: add netlink RTM_SETLINK command Laurent Vivier
2020-07-14  7:32 ` [PULL 6/7] linux-user: fix the errno value in print_syscall_err() Laurent Vivier
2020-07-14  7:32 ` [PULL 7/7] linux-user: fix print_syscall_err() when syscall returned value is negative Laurent Vivier
2020-07-14  7:59 ` [PULL 0/7] Linux user for 5.1 patches no-reply
2020-07-14 20:21 ` Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).