[Qemu-devel] [PATCH v3 00/13] linux-user: strace improvements

[Qemu-devel] [PATCH v3 00/13] linux-user: strace improvements

[Philippe Mathieu-Daudé]

Hi Laurent,

Few patches I'v been writting while trying to figure out this issue:
http://lists.nongnu.org/archive/html/qemu-arm/2018-01/msg00514.html

As usual with linux-user files, this series will trigger some checkpatch
benign warnings.

Regards,

Phil.

Since v2:
- display invalid pointer in print_timeval() and print_timezone()
- do not display gettimeofday() arguments

Since v1:
- addressed Laurent comments
- added 'last' argument to print_sockaddr()
- reordered series, so patches already correct can get applied directly
- dropped "linux-user/syscall: simplify recvfrom()" for now

v1: http://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg05855.html
v2: http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg08216.html

$ git backport-diff
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/13:[----] [--] 'linux-user/syscall: Verify recvfrom(addr) is user-writable'
002/13:[----] [--] 'linux-user/strace: Improve capget()/capset() output'
003/13:[down] 'linux-user/strace: Display invalid pointer in print_timeval()'
004/13:[0001] [FC] 'linux-user/strace: Add print_timezone()'
005/13:[down] 'linux-user/strace: Improve settimeofday()'
006/13:[----] [--] 'linux-user/strace: Dump AF_NETLINK sockaddr content'
007/13:[----] [--] 'linux-user/strace: Improve recvmsg() output'
008/13:[----] [--] 'linux-user/strace: Improve bind() output'
009/13:[----] [--] 'linux-user/strace: improve sendto() output'
010/13:[----] [--] 'linux-user/strace: Let print_sockaddr() have a 'last' argument'
011/13:[----] [--] 'linux-user/strace: Add print_sockaddr_ptr() to handle plain/pointer addrlen'
012/13:[----] [--] 'linux-user/strace: Improve getsockname() output'
013/13:[----] [--] 'linux-user/strace: Improve recvfrom() output'

Philippe Mathieu-Daudé (13):
  linux-user/syscall: Verify recvfrom(addr) is user-writable
  linux-user/strace: Improve capget()/capset() output
  linux-user/strace: Display invalid pointer in print_timeval()
  linux-user/strace: Add print_timezone()
  linux-user/strace: Improve settimeofday()
  linux-user/strace: Dump AF_NETLINK sockaddr content
  linux-user/strace: Improve recvmsg() output
  linux-user/strace: Improve bind() output
  linux-user/strace: improve sendto() output
  linux-user/strace: Let print_sockaddr() have a 'last' argument
  linux-user/strace: Add print_sockaddr_ptr() to handle plain/pointer addrlen
  linux-user/strace: Improve getsockname() output
  linux-user/strace: Improve recvfrom() output

 linux-user/syscall_defs.h |   7 ++
 linux-user/strace.c       | 164 ++++++++++++++++++++++++++++++++++++--
 linux-user/syscall.c      |  11 ++-
 linux-user/strace.list    |  16 ++--
 4 files changed, 182 insertions(+), 16 deletions(-)

-- 
2.18.0

[Qemu-devel] [PATCH v3 01/13] linux-user/syscall: Verify recvfrom(addr) is user-writable

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-By: Guido Günther <agx@sigxcpu.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/syscall.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 2117fb13b4..ad40682cee 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -4154,6 +4154,11 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
             ret = -TARGET_EINVAL;
             goto fail;
         }
+        if (!access_ok(VERIFY_WRITE, target_addr, addrlen)) {
+            ret = -TARGET_EFAULT;
+            goto fail;
+        }
+
         addr = alloca(addrlen);
         ret = get_errno(safe_recvfrom(fd, host_msg, len, flags,
                                       addr, &addrlen));
-- 
2.18.0

[Qemu-devel] [PATCH v3 02/13] linux-user/strace: Improve capget()/capset() output

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-By: Guido Günther <agx@sigxcpu.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/strace.list | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/linux-user/strace.list b/linux-user/strace.list
index 2bc5ba04d4..afe4db07f3 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -59,10 +59,10 @@
 { TARGET_NR_cacheflush, "cacheflush" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_capget
-{ TARGET_NR_capget, "capget" , NULL, NULL, NULL },
+{ TARGET_NR_capget, "capget" , "%s(%p,%p)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_capset
-{ TARGET_NR_capset, "capset" , NULL, NULL, NULL },
+{ TARGET_NR_capset, "capset" , "%s(%p,%p)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_chdir
 { TARGET_NR_chdir, "chdir" , NULL, print_chdir, NULL },
-- 
2.18.0

[Qemu-devel] [PATCH v3 05/13] linux-user/strace: Improve settimeofday()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-By: Guido Günther <agx@sigxcpu.org>
---
 linux-user/strace.c    | 13 +++++++++++++
 linux-user/strace.list |  2 +-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 49bdee6d83..8a21d3bcfc 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1502,6 +1502,19 @@ print_futimesat(const struct syscallname *name,
 }
 #endif
 
+#ifdef TARGET_NR_settimeofday
+static void
+print_settimeofday(const struct syscallname *name,
+                abi_long arg0, abi_long arg1, abi_long arg2,
+                abi_long arg3, abi_long arg4, abi_long arg5)
+{
+    print_syscall_prologue(name);
+    print_timeval(arg0, 0);
+    print_timezone(arg1, 1);
+    print_syscall_epilogue(name);
+}
+#endif
+
 #ifdef TARGET_NR_link
 static void
 print_link(const struct syscallname *name,
diff --git a/linux-user/strace.list b/linux-user/strace.list
index afe4db07f3..90cf7e338a 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -1345,7 +1345,7 @@
 { TARGET_NR_set_tid_address, "set_tid_address" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_settimeofday
-{ TARGET_NR_settimeofday, "settimeofday" , NULL, NULL, NULL },
+{ TARGET_NR_settimeofday, "settimeofday" , NULL, print_settimeofday, NULL },
 #endif
 #ifdef TARGET_NR_setuid
 { TARGET_NR_setuid, "setuid" , NULL, NULL, NULL },
-- 
2.18.0

[Qemu-devel] [PATCH v3 06/13] linux-user/strace: Dump AF_NETLINK sockaddr content

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-By: Guido Günther <agx@sigxcpu.org>
---
 linux-user/syscall_defs.h |  7 +++++++
 linux-user/strace.c       | 34 ++++++++++++++++++++++++++++++++++
 linux-user/syscall.c      |  6 ++++--
 3 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 40bb60ef4c..5a0b76721f 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -152,6 +152,13 @@ struct target_sockaddr_un {
     uint8_t sun_path[108];
 };
 
+struct target_sockaddr_nl {
+    uint16_t nl_family;     /* AF_NETLINK */
+    uint16_t __pad;
+    uint32_t nl_pid;
+    uint32_t nl_groups;
+};
+
 struct target_in_addr {
     uint32_t s_addr; /* big endian */
 };
diff --git a/linux-user/strace.c b/linux-user/strace.c
index 8a21d3bcfc..77e36467cd 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -8,6 +8,7 @@
 #include <arpa/inet.h>
 #include <netinet/tcp.h>
 #include <linux/if_packet.h>
+#include <linux/netlink.h>
 #include <sched.h>
 #include "qemu.h"
 
@@ -398,6 +399,12 @@ print_sockaddr(abi_ulong addr, abi_long addrlen)
             gemu_log("}");
             break;
         }
+        case AF_NETLINK: {
+            struct target_sockaddr_nl *nl = (struct target_sockaddr_nl *)sa;
+            gemu_log("{nl_family=AF_NETLINK,nl_pid=%u,nl_groups=%u}",
+                     nl->nl_pid, nl->nl_groups);
+            break;
+        }
         default:
             gemu_log("{sa_family=%d, sa_data={", sa->sa_family);
             for (i = 0; i < 13; i++) {
@@ -424,6 +431,9 @@ print_socket_domain(int domain)
     case PF_INET:
         gemu_log("PF_INET");
         break;
+    case PF_NETLINK:
+        gemu_log("PF_NETLINK");
+        break;
     case PF_PACKET:
         gemu_log("PF_PACKET");
         break;
@@ -473,6 +483,30 @@ print_socket_protocol(int domain, int type, int protocol)
         return;
     }
 
+    if (domain == AF_NETLINK) {
+        switch (protocol) {
+        case NETLINK_ROUTE:
+            gemu_log("NETLINK_ROUTE");
+            break;
+        case NETLINK_AUDIT:
+            gemu_log("NETLINK_AUDIT");
+            break;
+        case NETLINK_NETFILTER:
+            gemu_log("NETLINK_NETFILTER");
+            break;
+        case NETLINK_RDMA:
+            gemu_log("NETLINK_RDMA");
+            break;
+        case NETLINK_CRYPTO:
+            gemu_log("NETLINK_CRYPTO");
+            break;
+        default:
+            gemu_log("%d", protocol);
+            break;
+        }
+        return;
+    }
+
     switch (protocol) {
     case IPPROTO_IP:
         gemu_log("IPPROTO_IP");
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ad40682cee..9a11f8c4a6 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1696,8 +1696,10 @@ static inline abi_long host_to_target_sockaddr(abi_ulong target_addr,
         sizeof(target_saddr->sa_family)) {
         target_saddr->sa_family = tswap16(addr->sa_family);
     }
-    if (addr->sa_family == AF_NETLINK && len >= sizeof(struct sockaddr_nl)) {
-        struct sockaddr_nl *target_nl = (struct sockaddr_nl *)target_saddr;
+    if (addr->sa_family == AF_NETLINK &&
+        len >= sizeof(struct target_sockaddr_nl)) {
+        struct target_sockaddr_nl *target_nl =
+               (struct target_sockaddr_nl *)target_saddr;
         target_nl->nl_pid = tswap32(target_nl->nl_pid);
         target_nl->nl_groups = tswap32(target_nl->nl_groups);
     } else if (addr->sa_family == AF_PACKET) {
-- 
2.18.0

[Qemu-devel] [PATCH v3 07/13] linux-user/strace: Improve recvmsg() output

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-By: Guido Günther <agx@sigxcpu.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/strace.list | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/strace.list b/linux-user/strace.list
index 90cf7e338a..31093c3371 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -1107,7 +1107,7 @@
 { TARGET_NR_recvmmsg, "recvmmsg" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_recvmsg
-{ TARGET_NR_recvmsg, "recvmsg" , NULL, NULL, NULL },
+{ TARGET_NR_recvmsg, "recvmsg" , "%s(%d,%p,%#x)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_remap_file_pages
 { TARGET_NR_remap_file_pages, "remap_file_pages" , NULL, NULL, NULL },
-- 
2.18.0

[Qemu-devel] [PATCH v3 10/13] linux-user/strace: Let print_sockaddr() have a 'last' argument

[Philippe Mathieu-Daudé]

If the format is not the syscall last argument, a comma is append.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 linux-user/strace.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 390a50d45a..85a1efef32 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -67,7 +67,7 @@ UNUSED static void print_timeval(abi_ulong, int);
 UNUSED static void print_timezone(abi_ulong, int);
 UNUSED static void print_number(abi_long, int);
 UNUSED static void print_signal(abi_ulong, int);
-UNUSED static void print_sockaddr(abi_ulong addr, abi_long addrlen);
+UNUSED static void print_sockaddr(abi_ulong addr, abi_long addrlen, int);
 UNUSED static void print_socket_domain(int domain);
 UNUSED static void print_socket_type(int type);
 UNUSED static void print_socket_protocol(int domain, int type, int protocol);
@@ -336,7 +336,7 @@ static void print_siginfo(const target_siginfo_t *tinfo)
 }
 
 static void
-print_sockaddr(abi_ulong addr, abi_long addrlen)
+print_sockaddr(abi_ulong addr, abi_long addrlen, int last)
 {
     struct target_sockaddr *sa;
     int i;
@@ -418,7 +418,7 @@ print_sockaddr(abi_ulong addr, abi_long addrlen)
     } else {
         print_raw_param("0x"TARGET_ABI_FMT_lx, addr, 0);
     }
-    gemu_log(", "TARGET_ABI_FMT_ld, addrlen);
+    gemu_log(", "TARGET_ABI_FMT_ld"%s", addrlen, get_comma(last));
 }
 
 static void
@@ -1658,7 +1658,7 @@ static void do_print_sockaddr(const char *name, abi_long arg1)
 
     gemu_log("%s(", name);
     print_raw_param(TARGET_ABI_FMT_ld, sockfd, 0);
-    print_sockaddr(addr, addrlen);
+    print_sockaddr(addr, addrlen, 0);
     gemu_log(")");
 }
 
@@ -1728,7 +1728,7 @@ static void do_print_msgaddr(const char *name, abi_long arg1)
     print_buf(msg, len, 0);
     print_raw_param(TARGET_ABI_FMT_ld, len, 0);
     print_flags(msg_flags, flags, 0);
-    print_sockaddr(addr, addrlen);
+    print_sockaddr(addr, addrlen, 0);
     gemu_log(")");
 }
 
@@ -1966,7 +1966,7 @@ print_bind(const struct syscallname *name,
 {
     print_syscall_prologue(name);
     print_raw_param("%d", arg0, 0);
-    print_sockaddr(arg1, arg2);
+    print_sockaddr(arg1, arg2, 1);
     print_syscall_epilogue(name);
 }
 #endif
@@ -1982,7 +1982,7 @@ print_sendto(const struct syscallname *name,
     print_buf(arg1, arg2, 0);
     print_raw_param(TARGET_ABI_FMT_ld, arg2, 0);
     print_flags(msg_flags, arg3, 0);
-    print_sockaddr(arg4, arg5);
+    print_sockaddr(arg4, arg5, 1);
     print_syscall_epilogue(name);
 }
 #endif
-- 
2.18.0

[Qemu-devel] [PATCH v3 11/13] linux-user/strace: Add print_sockaddr_ptr() to handle plain/pointer addrlen

[Philippe Mathieu-Daudé]

While very similar to send()/recv(), the format used by
sendto()/recvfrom() is slightly different: the 'addrlen'
is not a plain sockaddr_t but a pointer to it.

Split the current function to handle both formats.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-By: Guido Günther <agx@sigxcpu.org>
---
 linux-user/strace.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 85a1efef32..9ab11059e4 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -68,6 +68,7 @@ UNUSED static void print_timezone(abi_ulong, int);
 UNUSED static void print_number(abi_long, int);
 UNUSED static void print_signal(abi_ulong, int);
 UNUSED static void print_sockaddr(abi_ulong addr, abi_long addrlen, int);
+UNUSED static void print_sockaddr_ptr(abi_ulong addr, abi_long addrlen_ptr, int);
 UNUSED static void print_socket_domain(int domain);
 UNUSED static void print_socket_type(int type);
 UNUSED static void print_socket_protocol(int domain, int type, int protocol);
@@ -336,7 +337,8 @@ static void print_siginfo(const target_siginfo_t *tinfo)
 }
 
 static void
-print_sockaddr(abi_ulong addr, abi_long addrlen, int last)
+print_sockaddr_ex(abi_ulong addr, abi_long addrlen,
+                  bool addrlen_is_ptr, int last)
 {
     struct target_sockaddr *sa;
     int i;
@@ -418,7 +420,29 @@ print_sockaddr(abi_ulong addr, abi_long addrlen, int last)
     } else {
         print_raw_param("0x"TARGET_ABI_FMT_lx, addr, 0);
     }
-    gemu_log(", "TARGET_ABI_FMT_ld"%s", addrlen, get_comma(last));
+    gemu_log(", %s"TARGET_ABI_FMT_ld"%s%s",
+             (addrlen_is_ptr ? "[" : ""), addrlen,
+             (addrlen_is_ptr ? "]" : ""), get_comma(last));
+}
+
+static void
+print_sockaddr(abi_ulong addr, abi_long addrlen, int last)
+{
+    print_sockaddr_ex(addr, addrlen, false, last);
+}
+
+static void
+print_sockaddr_ptr(abi_ulong addr, abi_long addrlen_ptr, int last)
+{
+    abi_ulong addrlen;
+
+    if (!addr) {
+        print_sockaddr_ex(0, 0, false, last);
+        return;
+    }
+
+    get_user_ual(addrlen, addrlen_ptr);
+    print_sockaddr_ex(addr, addrlen, true, last);
 }
 
 static void
-- 
2.18.0

[Qemu-devel] [PATCH v3 12/13] linux-user/strace: Improve getsockname() output

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-By: Guido Günther <agx@sigxcpu.org>
---
 linux-user/strace.c    | 13 +++++++++++++
 linux-user/strace.list |  2 +-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 9ab11059e4..f80d655835 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1995,6 +1995,19 @@ print_bind(const struct syscallname *name,
 }
 #endif
 
+#if defined(TARGET_NR_getsockname)
+static void
+print_getsockname(const struct syscallname *name,
+                  abi_long arg0, abi_long arg1, abi_long arg2,
+                  abi_long arg3, abi_long arg4, abi_long arg5)
+{
+    print_syscall_prologue(name);
+    print_raw_param("%d", arg0, 0);
+    print_sockaddr_ptr(arg1, arg2, 1);
+    print_syscall_epilogue(name);
+}
+#endif
+
 #if defined(TARGET_NR_sendto)
 static void
 print_sendto(const struct syscallname *name,
diff --git a/linux-user/strace.list b/linux-user/strace.list
index 06597fd58c..019bf54850 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -371,7 +371,7 @@
 { TARGET_NR_getsid, "getsid" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_getsockname
-{ TARGET_NR_getsockname, "getsockname" , NULL, NULL, NULL },
+{ TARGET_NR_getsockname, "getsockname" , NULL, print_getsockname, NULL },
 #endif
 #ifdef TARGET_NR_getsockopt
 { TARGET_NR_getsockopt, "getsockopt" , NULL, NULL, NULL },
-- 
2.18.0

[Qemu-devel] [PATCH v3 13/13] linux-user/strace: Improve recvfrom() output

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-By: Guido Günther <agx@sigxcpu.org>
---
 linux-user/strace.c    | 16 ++++++++++++++++
 linux-user/strace.list |  2 +-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index f80d655835..a85b4a10e5 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -2008,6 +2008,22 @@ print_getsockname(const struct syscallname *name,
 }
 #endif
 
+#if defined(TARGET_NR_recvfrom)
+static void
+print_recvfrom(const struct syscallname *name,
+               abi_long arg0, abi_long arg1, abi_long arg2,
+               abi_long arg3, abi_long arg4, abi_long arg5)
+{
+    print_syscall_prologue(name);
+    print_raw_param("%d", arg0, 0);
+    print_buf(arg1, arg2, 0);
+    print_raw_param(TARGET_ABI_FMT_ld, arg2, 0);
+    print_flags(msg_flags, arg3, 0);
+    print_sockaddr_ptr(arg4, arg5, 1);
+    print_syscall_epilogue(name);
+}
+#endif
+
 #if defined(TARGET_NR_sendto)
 static void
 print_sendto(const struct syscallname *name,
diff --git a/linux-user/strace.list b/linux-user/strace.list
index 019bf54850..82012353f6 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -1101,7 +1101,7 @@
 { TARGET_NR_recv, "recv" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_recvfrom
-{ TARGET_NR_recvfrom, "recvfrom" , NULL, NULL, NULL },
+{ TARGET_NR_recvfrom, "recvfrom" , NULL, print_recvfrom, NULL },
 #endif
 #ifdef TARGET_NR_recvmmsg
 { TARGET_NR_recvmmsg, "recvmmsg" , NULL, NULL, NULL },
-- 
2.18.0

Re: [Qemu-devel] [PATCH v3 03/13] linux-user/strace: Display invalid pointer in print_timeval()

[Laurent Vivier]

Le 02/07/2018 à 19:50, Philippe Mathieu-Daudé a écrit :
> Suggested-by: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  linux-user/strace.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index bd897a3f20..311e63ef75 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -1173,8 +1173,10 @@ print_timeval(abi_ulong tv_addr, int last)
>          struct target_timeval *tv;
>  
>          tv = lock_user(VERIFY_READ, tv_addr, sizeof(*tv), 1);
> -        if (!tv)
> +        if (!tv) {
> +            print_pointer(tv, last);

tv is NULL here. we want to print the value of tv_addr.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v3 04/13] linux-user/strace: Add print_timezone()

[Laurent Vivier]

Le 02/07/2018 à 19:50, Philippe Mathieu-Daudé a écrit :
> Suggested-by: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  linux-user/strace.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 311e63ef75..49bdee6d83 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -63,6 +63,7 @@ UNUSED static void print_string(abi_long, int);
>  UNUSED static void print_buf(abi_long addr, abi_long len, int last);
>  UNUSED static void print_raw_param(const char *, abi_long, int);
>  UNUSED static void print_timeval(abi_ulong, int);
> +UNUSED static void print_timezone(abi_ulong, int);
>  UNUSED static void print_number(abi_long, int);
>  UNUSED static void print_signal(abi_ulong, int);
>  UNUSED static void print_sockaddr(abi_ulong addr, abi_long addrlen);
> @@ -1184,6 +1185,26 @@ print_timeval(abi_ulong tv_addr, int last)
>          gemu_log("NULL%s", get_comma(last));
>  }
>  
> +static void
> +print_timezone(abi_ulong tz_addr, int last)
> +{
> +    if (tz_addr) {
> +        struct target_timezone *tz;
> +
> +        tz = lock_user(VERIFY_READ, tz_addr, sizeof(*tz), 1);
> +        if (!tz) {
> +            print_pointer(tz, last);

print_pointer(tz_addr, last);

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v3 05/13] linux-user/strace: Improve settimeofday()

[Laurent Vivier]

Le 02/07/2018 à 19:50, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Tested-By: Guido Günther <agx@sigxcpu.org>
> ---
>  linux-user/strace.c    | 13 +++++++++++++
>  linux-user/strace.list |  2 +-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [Qemu-devel] [PATCH v3 03/13] linux-user/strace: Display invalid pointer in print_timeval()

[Philippe Mathieu-Daudé]

On 07/02/2018 03:18 PM, Laurent Vivier wrote:
> Le 02/07/2018 à 19:50, Philippe Mathieu-Daudé a écrit :
>> Suggested-by: Laurent Vivier <laurent@vivier.eu>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  linux-user/strace.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/linux-user/strace.c b/linux-user/strace.c
>> index bd897a3f20..311e63ef75 100644
>> --- a/linux-user/strace.c
>> +++ b/linux-user/strace.c
>> @@ -1173,8 +1173,10 @@ print_timeval(abi_ulong tv_addr, int last)
>>          struct target_timeval *tv;
>>  
>>          tv = lock_user(VERIFY_READ, tv_addr, sizeof(*tv), 1);
>> -        if (!tv)
>> +        if (!tv) {
>> +            print_pointer(tv, last);
> 
> tv is NULL here. we want to print the value of tv_addr.

Oops sorry I need to keep focused!

Re: [Qemu-devel] [PATCH v3 08/13] linux-user/strace: Improve bind() output

[Laurent Vivier]

Le 02/07/2018 à 19:50, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Tested-By: Guido Günther <agx@sigxcpu.org>
> ---
>  linux-user/strace.c    | 13 +++++++++++++
>  linux-user/strace.list |  2 +-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 77e36467cd..b8e585a87d 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -1958,6 +1958,19 @@ print_socketcall(const struct syscallname *name,
>  }
>  #endif
>  
> +#if defined(TARGET_NR_bind)
> +static void
> +print_bind(const struct syscallname *name,
> +           abi_long arg0, abi_long arg1, abi_long arg2,
> +           abi_long arg3, abi_long arg4, abi_long arg5)
> +{
> +    print_syscall_prologue(name);
> +    print_raw_param("%d", arg0, 0);

As arg0 is abi_long I think you should use TARGET_ABI_FMT_ld.
(see do_print_sockaddr()).

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v3 09/13] linux-user/strace: improve sendto() output

[Laurent Vivier]

Le 02/07/2018 à 19:50, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Tested-By: Guido Günther <agx@sigxcpu.org>
> ---
>  linux-user/strace.c    | 16 ++++++++++++++++
>  linux-user/strace.list |  2 +-
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index b8e585a87d..390a50d45a 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -1971,6 +1971,22 @@ print_bind(const struct syscallname *name,
>  }
>  #endif
>  
> +#if defined(TARGET_NR_sendto)
> +static void
> +print_sendto(const struct syscallname *name,
> +             abi_long arg0, abi_long arg1, abi_long arg2,
> +             abi_long arg3, abi_long arg4, abi_long arg5)
> +{
> +    print_syscall_prologue(name);
> +    print_raw_param("%d", arg0, 0);

TARGET_ABI_FMT_ld

otherwise:

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [Qemu-devel] [PATCH v3 10/13] linux-user/strace: Let print_sockaddr() have a 'last' argument

[Laurent Vivier]

Le 02/07/2018 à 19:50, Philippe Mathieu-Daudé a écrit :
> If the format is not the syscall last argument, a comma is append.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  linux-user/strace.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [Qemu-devel] [PATCH v3 13/13] linux-user/strace: Improve recvfrom() output

[Laurent Vivier]

Le 02/07/2018 à 19:50, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Tested-By: Guido Günther <agx@sigxcpu.org>
> ---
>  linux-user/strace.c    | 16 ++++++++++++++++
>  linux-user/strace.list |  2 +-
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index f80d655835..a85b4a10e5 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -2008,6 +2008,22 @@ print_getsockname(const struct syscallname *name,
>  }
>  #endif
>  
> +#if defined(TARGET_NR_recvfrom)
> +static void
> +print_recvfrom(const struct syscallname *name,
> +               abi_long arg0, abi_long arg1, abi_long arg2,
> +               abi_long arg3, abi_long arg4, abi_long arg5)
> +{
> +    print_syscall_prologue(name);
> +    print_raw_param("%d", arg0, 0);
> +    print_buf(arg1, arg2, 0);

The content of the buffer is not relevant here as it is displayed before
the data are received. I think you should only print the pointer and the
length.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v3 12/13] linux-user/strace: Improve getsockname() output

[Laurent Vivier]

Le 02/07/2018 à 19:50, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Tested-By: Guido Günther <agx@sigxcpu.org>
> ---
>  linux-user/strace.c    | 13 +++++++++++++
>  linux-user/strace.list |  2 +-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 9ab11059e4..f80d655835 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -1995,6 +1995,19 @@ print_bind(const struct syscallname *name,
>  }
>  #endif
>  
> +#if defined(TARGET_NR_getsockname)
> +static void
> +print_getsockname(const struct syscallname *name,
> +                  abi_long arg0, abi_long arg1, abi_long arg2,
> +                  abi_long arg3, abi_long arg4, abi_long arg5)
> +{
> +    print_syscall_prologue(name);
> +    print_raw_param("%d", arg0, 0);

TARGET_AB_FMT_ld

otherwise:

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [Qemu-devel] [PATCH v3 11/13] linux-user/strace: Add print_sockaddr_ptr() to handle plain/pointer addrlen

[Laurent Vivier]

Le 02/07/2018 à 19:50, Philippe Mathieu-Daudé a écrit :
> While very similar to send()/recv(), the format used by
> sendto()/recvfrom() is slightly different: the 'addrlen'
> is not a plain sockaddr_t but a pointer to it.
> 
> Split the current function to handle both formats.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Tested-By: Guido Günther <agx@sigxcpu.org>
> ---
>  linux-user/strace.c | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
> 

In the following patches I think you can use do_print_sockaddr() and
don't have to define print_sockaddr_ptr()

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v3 06/13] linux-user/strace: Dump AF_NETLINK sockaddr content

[Laurent Vivier]

Le 02/07/2018 à 19:50, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Tested-By: Guido Günther <agx@sigxcpu.org>
> ---
>  linux-user/syscall_defs.h |  7 +++++++
>  linux-user/strace.c       | 34 ++++++++++++++++++++++++++++++++++
>  linux-user/syscall.c      |  6 ++++--
>  3 files changed, 45 insertions(+), 2 deletions(-)

Could you split this patch in two parts:
- one to introduce target_sockaddr_nl and modify syscall.c
- one to add the strace part

> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 40bb60ef4c..5a0b76721f 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -152,6 +152,13 @@ struct target_sockaddr_un {
>      uint8_t sun_path[108];
>  };
>  
> +struct target_sockaddr_nl {
> +    uint16_t nl_family;     /* AF_NETLINK */
> +    uint16_t __pad;
> +    uint32_t nl_pid;
> +    uint32_t nl_groups;
> +};
> +
>  struct target_in_addr {
>      uint32_t s_addr; /* big endian */
>  };
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 8a21d3bcfc..77e36467cd 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -8,6 +8,7 @@
>  #include <arpa/inet.h>
>  #include <netinet/tcp.h>
>  #include <linux/if_packet.h>
> +#include <linux/netlink.h>
>  #include <sched.h>
>  #include "qemu.h"
>  
> @@ -398,6 +399,12 @@ print_sockaddr(abi_ulong addr, abi_long addrlen)
>              gemu_log("}");
>              break;
>          }
> +        case AF_NETLINK: {
> +            struct target_sockaddr_nl *nl = (struct target_sockaddr_nl *)sa;
> +            gemu_log("{nl_family=AF_NETLINK,nl_pid=%u,nl_groups=%u}",
> +                     nl->nl_pid, nl->nl_groups);
> +            break;
> +        }
>          default:
>              gemu_log("{sa_family=%d, sa_data={", sa->sa_family);
>              for (i = 0; i < 13; i++) {
> @@ -424,6 +431,9 @@ print_socket_domain(int domain)
>      case PF_INET:
>          gemu_log("PF_INET");
>          break;
> +    case PF_NETLINK:
> +        gemu_log("PF_NETLINK");
> +        break;
>      case PF_PACKET:
>          gemu_log("PF_PACKET");
>          break;
> @@ -473,6 +483,30 @@ print_socket_protocol(int domain, int type, int protocol)
>          return;
>      }
>  
> +    if (domain == AF_NETLINK) {

domain uses PF_NETLINK

> +        switch (protocol) {
> +        case NETLINK_ROUTE:
> +            gemu_log("NETLINK_ROUTE");
> +            break;
> +        case NETLINK_AUDIT:
> +            gemu_log("NETLINK_AUDIT");
> +            break;
> +        case NETLINK_NETFILTER:
> +            gemu_log("NETLINK_NETFILTER");
> +            break;
> +        case NETLINK_RDMA:
> +            gemu_log("NETLINK_RDMA");
> +            break;
> +        case NETLINK_CRYPTO:
> +            gemu_log("NETLINK_CRYPTO");
> +            break;
> +        default:
> +            gemu_log("%d", protocol);
> +            break;
> +        }

add NETLINK_KOBJECT_UEVENT, it is managed in syscall.c

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v3 01/13] linux-user/syscall: Verify recvfrom(addr) is user-writable

[Laurent Vivier]

Le 02/07/2018 à 19:50, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Tested-By: Guido Günther <agx@sigxcpu.org>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  linux-user/syscall.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 2117fb13b4..ad40682cee 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -4154,6 +4154,11 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
>              ret = -TARGET_EINVAL;
>              goto fail;
>          }
> +        if (!access_ok(VERIFY_WRITE, target_addr, addrlen)) {
> +            ret = -TARGET_EFAULT;
> +            goto fail;
> +        }
> +
>          addr = alloca(addrlen);
>          ret = get_errno(safe_recvfrom(fd, host_msg, len, flags,
>                                        addr, &addrlen));
> 

This patch breaks the test recvfrom01 of the Linux Test Project.

Laurent

Re: [Qemu-devel] [PATCH v3 01/13] linux-user/syscall: Verify recvfrom(addr) is user-writable

[Philippe Mathieu-Daudé]

On 07/03/2018 11:55 AM, Laurent Vivier wrote:
> Le 02/07/2018 à 19:50, Philippe Mathieu-Daudé a écrit :
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> Tested-By: Guido Günther <agx@sigxcpu.org>
>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>> ---
>>  linux-user/syscall.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 2117fb13b4..ad40682cee 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -4154,6 +4154,11 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
>>              ret = -TARGET_EINVAL;
>>              goto fail;
>>          }
>> +        if (!access_ok(VERIFY_WRITE, target_addr, addrlen)) {
>> +            ret = -TARGET_EFAULT;
>> +            goto fail;
>> +        }
>> +
>>          addr = alloca(addrlen);
>>          ret = get_errno(safe_recvfrom(fd, host_msg, len, flags,
>>                                        addr, &addrlen));
>>
> 
> This patch breaks the test recvfrom01 of the Linux Test Project.

OK :(

I just sent v4, can you simply drop this patch?
I'll now look at it after 3.0.

Thanks,

Phil.

Re: [Qemu-devel] [PATCH v3 01/13] linux-user/syscall: Verify recvfrom(addr) is user-writable

[Laurent Vivier]

Le 03/07/2018 à 17:39, Philippe Mathieu-Daudé a écrit :
> On 07/03/2018 11:55 AM, Laurent Vivier wrote:
>> Le 02/07/2018 à 19:50, Philippe Mathieu-Daudé a écrit :
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> Tested-By: Guido Günther <agx@sigxcpu.org>
>>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>>> ---
>>>  linux-user/syscall.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>> index 2117fb13b4..ad40682cee 100644
>>> --- a/linux-user/syscall.c
>>> +++ b/linux-user/syscall.c
>>> @@ -4154,6 +4154,11 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
>>>              ret = -TARGET_EINVAL;
>>>              goto fail;
>>>          }
>>> +        if (!access_ok(VERIFY_WRITE, target_addr, addrlen)) {
>>> +            ret = -TARGET_EFAULT;
>>> +            goto fail;
>>> +        }
>>> +
>>>          addr = alloca(addrlen);
>>>          ret = get_errno(safe_recvfrom(fd, host_msg, len, flags,
>>>                                        addr, &addrlen));
>>>
>>
>> This patch breaks the test recvfrom01 of the Linux Test Project.
> 
> OK :(
> 
> I just sent v4, can you simply drop this patch?
> I'll now look at it after 3.0.

Not enough time to add more patches to my pull request, sorry.

I'll queue them in my branch for the next release.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v3 01/13] linux-user/syscall: Verify recvfrom(addr) is user-writable

[Philippe Mathieu-Daudé]

On 07/03/2018 01:38 PM, Laurent Vivier wrote:
> Le 03/07/2018 à 17:39, Philippe Mathieu-Daudé a écrit :
>> On 07/03/2018 11:55 AM, Laurent Vivier wrote:
>>> Le 02/07/2018 à 19:50, Philippe Mathieu-Daudé a écrit :
>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>> Tested-By: Guido Günther <agx@sigxcpu.org>
>>>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>>>> ---
>>>>  linux-user/syscall.c | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>>> index 2117fb13b4..ad40682cee 100644
>>>> --- a/linux-user/syscall.c
>>>> +++ b/linux-user/syscall.c
>>>> @@ -4154,6 +4154,11 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
>>>>              ret = -TARGET_EINVAL;
>>>>              goto fail;
>>>>          }
>>>> +        if (!access_ok(VERIFY_WRITE, target_addr, addrlen)) {
>>>> +            ret = -TARGET_EFAULT;
>>>> +            goto fail;
>>>> +        }
>>>> +
>>>>          addr = alloca(addrlen);
>>>>          ret = get_errno(safe_recvfrom(fd, host_msg, len, flags,
>>>>                                        addr, &addrlen));
>>>>
>>>
>>> This patch breaks the test recvfrom01 of the Linux Test Project.
>>
>> OK :(
>>
>> I just sent v4, can you simply drop this patch?
>> I'll now look at it after 3.0.
> 
> Not enough time to add more patches to my pull request, sorry.
> 
> I'll queue them in my branch for the next release.

Fair enough :)

Thanks!

Phil.