[Qemu-devel] [PATCH 00/11] linux-user: improve NETLINK strace

[Qemu-devel] [PATCH 00/11] linux-user: improve NETLINK strace

[Philippe Mathieu-Daudé]

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

Regards,

Phil.

Philippe Mathieu-Daudé (11):
  linux-user/strace: dump AF_NETLINK sockaddr content
  linux-user/strace: improve sendto() output
  linux-user/strace: add print_sockaddr_ptr() to handle plain/pointer addrlen
  linux-user/strace: improve recvfrom() output
  linux-user/strace: improve getsockname() output
  linux-user/strace: improve recvmsg() output
  linux-user/strace: improve bind() output
  linux-user/strace: improve gettimeofday() output
  linux-user/strace: improve capget()/capset() output
  linux-user/syscall: verify recvfrom(addr) is user-writable
  linux-user/syscall: simplify recvfrom()

 linux-user/syscall_defs.h |   7 +++
 linux-user/strace.c       | 122 +++++++++++++++++++++++++++++++++++++++++++++-
 linux-user/syscall.c      |  16 +++---
 linux-user/strace.list    |  16 +++---
 4 files changed, 144 insertions(+), 17 deletions(-)

-- 
2.15.1

[Qemu-devel] [RFC PATCH 01/11] linux-user/strace: dump AF_NETLINK sockaddr content

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
please double check __pad and ntohl()

 linux-user/syscall_defs.h |  7 +++++++
 linux-user/strace.c       | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index bec3680b94..550e7d2939 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -151,6 +151,13 @@ struct target_sockaddr_un {
     uint8_t sun_path[108];
 };
 
+struct target_sockaddr_nl {
+    uint16_t nl_family;     /* AF_NETLINK */
+    int16_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 bd897a3f20..7eb5e2ab48 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"
 
@@ -397,6 +398,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}",
+                     ntohl(nl->nl_pid), ntohl(nl->nl_groups));
+            break;
+        }
         default:
             gemu_log("{sa_family=%d, sa_data={", sa->sa_family);
             for (i = 0; i < 13; i++) {
@@ -423,6 +430,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;
@@ -472,6 +482,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");
-- 
2.15.1

[Qemu-devel] [PATCH 02/11] linux-user/strace: improve sendto() output

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.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 7eb5e2ab48..e7272f4ede 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1922,6 +1922,22 @@ print_socketcall(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(TARGET_ABI_FMT_ld, arg0, 0);
+    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_syscall_epilogue(name);
+}
+#endif
+
 #if defined(TARGET_NR_stat) || defined(TARGET_NR_stat64) || \
     defined(TARGET_NR_lstat) || defined(TARGET_NR_lstat64)
 static void
diff --git a/linux-user/strace.list b/linux-user/strace.list
index a91e33f7e5..47669a2f50 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -1248,7 +1248,7 @@
 { TARGET_NR_sendmsg, "sendmsg" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_sendto
-{ TARGET_NR_sendto, "sendto" , NULL, NULL, NULL },
+{ TARGET_NR_sendto, "sendto" , NULL, print_sendto, NULL },
 #endif
 #ifdef TARGET_NR_setdomainname
 { TARGET_NR_setdomainname, "setdomainname" , NULL, NULL, NULL },
-- 
2.15.1

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

[Philippe Mathieu-Daudé]

since this argument differs between sendto()/recvfrom()

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

diff --git a/linux-user/strace.c b/linux-user/strace.c
index e7272f4ede..9726d9b378 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -335,12 +335,15 @@ static void print_siginfo(const target_siginfo_t *tinfo)
 }
 
 static void
-print_sockaddr(abi_ulong addr, abi_long addrlen)
+print_sockaddr_ptr(abi_ulong addr, abi_long addrlen, bool addrlen_ptr)
 {
     struct target_sockaddr *sa;
     int i;
     int sa_family;
 
+    if (addrlen_ptr) {
+        get_user_ual(addrlen, addrlen);
+    }
     sa = lock_user(VERIFY_READ, addr, addrlen, 1);
     if (sa) {
         sa_family = tswap16(sa->sa_family);
@@ -417,7 +420,17 @@ 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);
+    if (addrlen_ptr) {
+        gemu_log(", ["TARGET_ABI_FMT_ld"]", addrlen);
+    } else {
+        gemu_log(", "TARGET_ABI_FMT_ld, addrlen);
+    }
+}
+
+static void
+print_sockaddr(abi_ulong addr, abi_long addrlen)
+{
+    print_sockaddr_ptr(addr, addrlen, false);
 }
 
 static void
-- 
2.15.1

[Qemu-devel] [PATCH 04/11] linux-user/strace: improve recvfrom() output

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.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 9726d9b378..3537a3ae92 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1935,6 +1935,22 @@ print_socketcall(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(TARGET_ABI_FMT_ld, 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, true);
+    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 47669a2f50..bf4192fb47 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.15.1

[Qemu-devel] [PATCH 05/11] linux-user/strace: improve getsockname() output

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.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 3537a3ae92..70ecb1fc98 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1935,6 +1935,19 @@ print_socketcall(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(TARGET_ABI_FMT_ld, arg0, 0);
+    print_sockaddr_ptr(arg1, arg2, true);
+    print_syscall_epilogue(name);
+}
+#endif
+
 #if defined(TARGET_NR_recvfrom)
 static void
 print_recvfrom(const struct syscallname *name,
diff --git a/linux-user/strace.list b/linux-user/strace.list
index bf4192fb47..947ff04eab 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.15.1

[Qemu-devel] [PATCH 06/11] linux-user/strace: improve recvmsg() output

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 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 947ff04eab..ae6dc8fecf 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.15.1

[Qemu-devel] [PATCH 07/11] linux-user/strace: improve bind() output

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.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 70ecb1fc98..4b8ab6bcfb 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1935,6 +1935,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(TARGET_ABI_FMT_ld, arg0, 0);
+    print_sockaddr(arg1, arg2);
+    print_syscall_epilogue(name);
+}
+#endif
+
 #if defined(TARGET_NR_getsockname)
 static void
 print_getsockname(const struct syscallname *name,
diff --git a/linux-user/strace.list b/linux-user/strace.list
index ae6dc8fecf..958d10d48f 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -41,7 +41,7 @@
 { TARGET_NR_bdflush, "bdflush" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_bind
-{ TARGET_NR_bind, "bind" , NULL, NULL, NULL },
+{ TARGET_NR_bind, "bind" , NULL, print_bind, NULL },
 #endif
 #ifdef TARGET_NR_bpf
 { TARGET_NR_bpf, "bpf" , NULL, NULL, NULL },
-- 
2.15.1

[Qemu-devel] [PATCH 08/11] linux-user/strace: improve gettimeofday() output

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.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 4b8ab6bcfb..b7c4cfae58 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1526,6 +1526,19 @@ print_futimesat(const struct syscallname *name,
 }
 #endif
 
+#ifdef TARGET_NR_gettimeofday
+static void
+print_gettimeofday(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_pointer(arg0, 0);
+    print_pointer(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 958d10d48f..f90c0e8ee4 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -384,7 +384,7 @@
 { TARGET_NR_gettid, "gettid" , "%s()", NULL, NULL },
 #endif
 #ifdef TARGET_NR_gettimeofday
-{ TARGET_NR_gettimeofday, "gettimeofday" , NULL, NULL, NULL },
+{ TARGET_NR_gettimeofday, "gettimeofday" , NULL, print_gettimeofday, NULL },
 #endif
 #ifdef TARGET_NR_getuid
 { TARGET_NR_getuid, "getuid" , "%s()", NULL, NULL },
-- 
2.15.1

[Qemu-devel] [PATCH 09/11] linux-user/strace: improve capget()/capset() output

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 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 f90c0e8ee4..f09234345f 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.15.1

[Qemu-devel] [PATCH 10/11] linux-user/syscall: verify recvfrom(addr) is user-writable

[Philippe Mathieu-Daudé]

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

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 11c9116c4a..b6b9beca5b 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -4040,6 +4040,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.15.1

[Qemu-devel] [PATCH 11/11] linux-user/syscall: simplify recvfrom()

[Philippe Mathieu-Daudé]

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

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index b6b9beca5b..e082af2911 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -4023,8 +4023,8 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
                             abi_ulong target_addr,
                             abi_ulong target_addrlen)
 {
-    socklen_t addrlen;
-    void *addr;
+    socklen_t addrlen = 0;
+    void *addr = NULL;
     void *host_msg;
     abi_long ret;
 
@@ -4046,12 +4046,9 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
         }
 
         addr = alloca(addrlen);
-        ret = get_errno(safe_recvfrom(fd, host_msg, len, flags,
-                                      addr, &addrlen));
-    } else {
-        addr = NULL; /* To keep compiler quiet.  */
-        ret = get_errno(safe_recvfrom(fd, host_msg, len, flags, NULL, 0));
     }
+    ret = get_errno(safe_recvfrom(fd, host_msg, len, flags, addr, &addrlen));
+
     if (!is_error(ret)) {
         if (fd_trans_host_to_target_data(fd)) {
             ret = fd_trans_host_to_target_data(fd)(host_msg, ret);
-- 
2.15.1

Re: [Qemu-devel] [PATCH 00/11] linux-user: improve NETLINK strace

[Guido Günther]

Hi,
On Wed, Jan 24, 2018 at 10:01:15AM -0300, Philippe Mathieu-Daudé wrote:
> 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

I can't comment code wise but it makes the -strace output much more
useful in this area.
 -- Guido

> 
> Regards,
> 
> Phil.
> 
> Philippe Mathieu-Daudé (11):
>   linux-user/strace: dump AF_NETLINK sockaddr content
>   linux-user/strace: improve sendto() output
>   linux-user/strace: add print_sockaddr_ptr() to handle plain/pointer addrlen
>   linux-user/strace: improve recvfrom() output
>   linux-user/strace: improve getsockname() output
>   linux-user/strace: improve recvmsg() output
>   linux-user/strace: improve bind() output
>   linux-user/strace: improve gettimeofday() output
>   linux-user/strace: improve capget()/capset() output
>   linux-user/syscall: verify recvfrom(addr) is user-writable
>   linux-user/syscall: simplify recvfrom()
> 
>  linux-user/syscall_defs.h |   7 +++
>  linux-user/strace.c       | 122 +++++++++++++++++++++++++++++++++++++++++++++-
>  linux-user/syscall.c      |  16 +++---
>  linux-user/strace.list    |  16 +++---
>  4 files changed, 144 insertions(+), 17 deletions(-)
> 
> -- 
> 2.15.1
> 

Re: [Qemu-devel] [PATCH 00/11] linux-user: improve NETLINK strace

[Laurent Vivier]

Le 24/01/2018 à 19:50, Guido Günther a écrit :
> Hi,
> On Wed, Jan 24, 2018 at 10:01:15AM -0300, Philippe Mathieu-Daudé wrote:
>> 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
> 
> I can't comment code wise but it makes the -strace output much more
> useful in this area.

If you have tested the patches and found they work well, you can add a:

Tested-by: .....

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 00/11] linux-user: improve NETLINK strace

[Guido Günther]

Hi,
On Wed, Jan 24, 2018 at 10:01:15AM -0300, Philippe Mathieu-Daudé wrote:
> 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

Whole series

Tested-By: Guido Günther <agx@sigxcpu.org>

> 
> Regards,
> 
> Phil.
> 
> Philippe Mathieu-Daudé (11):
>   linux-user/strace: dump AF_NETLINK sockaddr content
>   linux-user/strace: improve sendto() output
>   linux-user/strace: add print_sockaddr_ptr() to handle plain/pointer addrlen
>   linux-user/strace: improve recvfrom() output
>   linux-user/strace: improve getsockname() output
>   linux-user/strace: improve recvmsg() output
>   linux-user/strace: improve bind() output
>   linux-user/strace: improve gettimeofday() output
>   linux-user/strace: improve capget()/capset() output
>   linux-user/syscall: verify recvfrom(addr) is user-writable
>   linux-user/syscall: simplify recvfrom()
> 
>  linux-user/syscall_defs.h |   7 +++
>  linux-user/strace.c       | 122 +++++++++++++++++++++++++++++++++++++++++++++-
>  linux-user/syscall.c      |  16 +++---
>  linux-user/strace.list    |  16 +++---
>  4 files changed, 144 insertions(+), 17 deletions(-)
> 
> -- 
> 2.15.1
> 

Re: [Qemu-devel] [PATCH 00/11] linux-user: improve NETLINK strace

[Philippe Mathieu-Daudé]

On Sun, Jan 28, 2018 at 8:12 AM, Guido Günther <agx@sigxcpu.org> wrote:
> Hi,
> On Wed, Jan 24, 2018 at 10:01:15AM -0300, Philippe Mathieu-Daudé wrote:
>> 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
>
> Whole series
>
> Tested-By: Guido Günther <agx@sigxcpu.org>

Thank you Guido!

Re: [Qemu-devel] [RFC PATCH 01/11] linux-user/strace: dump AF_NETLINK sockaddr content

[Laurent Vivier]

Le 24/01/2018 à 14:01, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> please double check __pad and ntohl()
> 
>  linux-user/syscall_defs.h |  7 +++++++
>  linux-user/strace.c       | 34 ++++++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index bec3680b94..550e7d2939 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -151,6 +151,13 @@ struct target_sockaddr_un {
>      uint8_t sun_path[108];
>  };
>  
> +struct target_sockaddr_nl {
> +    uint16_t nl_family;     /* AF_NETLINK */
> +    int16_t __pad;

netlink.h uses an unsigned type here.

> +    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 bd897a3f20..7eb5e2ab48 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"
>  
> @@ -397,6 +398,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}",
> +                     ntohl(nl->nl_pid), ntohl(nl->nl_groups));

Both sides of the netlink pipe are local and have the same endianness,
so I don't think we need the ntohl() here. Moreover, I didn't find any
endianness change in the kernel for them.

Thanks,
Laurent

Re: [Qemu-devel] [RFC PATCH 01/11] linux-user/strace: dump AF_NETLINK sockaddr content

[Philippe Mathieu-Daudé]

Hi Laurent,

On 02/15/2018 12:17 PM, Laurent Vivier wrote:
> Le 24/01/2018 à 14:01, Philippe Mathieu-Daudé a écrit :
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> please double check __pad and ntohl()
>>
>>  linux-user/syscall_defs.h |  7 +++++++
>>  linux-user/strace.c       | 34 ++++++++++++++++++++++++++++++++++
>>  2 files changed, 41 insertions(+)
>>
>> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
>> index bec3680b94..550e7d2939 100644
>> --- a/linux-user/syscall_defs.h
>> +++ b/linux-user/syscall_defs.h
>> @@ -151,6 +151,13 @@ struct target_sockaddr_un {
>>      uint8_t sun_path[108];
>>  };
>>  
>> +struct target_sockaddr_nl {
>> +    uint16_t nl_family;     /* AF_NETLINK */
>> +    int16_t __pad;
> 
> netlink.h uses an unsigned type here.

This is padding, do you prefer char pad[2]?

> 
>> +    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 bd897a3f20..7eb5e2ab48 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"
>>  
>> @@ -397,6 +398,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}",
>> +                     ntohl(nl->nl_pid), ntohl(nl->nl_groups));
> 
> Both sides of the netlink pipe are local and have the same endianness,
> so I don't think we need the ntohl() here. Moreover, I didn't find any
> endianness change in the kernel for them.

I had the same feeling but didn't check the kernel :\ since I was unsure
I used ntohl(), I'll remove it and respin. Thanks to verify!

Regards,

Phil.

> 
> Thanks,
> Laurent
> 

Re: [Qemu-devel] [RFC PATCH 01/11] linux-user/strace: dump AF_NETLINK sockaddr content

[Laurent Vivier]

Le 15/02/2018 à 16:28, Philippe Mathieu-Daudé a écrit :
> Hi Laurent,
> 
> On 02/15/2018 12:17 PM, Laurent Vivier wrote:
>> Le 24/01/2018 à 14:01, Philippe Mathieu-Daudé a écrit :
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>> please double check __pad and ntohl()
>>>
>>>  linux-user/syscall_defs.h |  7 +++++++
>>>  linux-user/strace.c       | 34 ++++++++++++++++++++++++++++++++++
>>>  2 files changed, 41 insertions(+)
>>>
>>> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
>>> index bec3680b94..550e7d2939 100644
>>> --- a/linux-user/syscall_defs.h
>>> +++ b/linux-user/syscall_defs.h
>>> @@ -151,6 +151,13 @@ struct target_sockaddr_un {
>>>      uint8_t sun_path[108];
>>>  };
>>>  
>>> +struct target_sockaddr_nl {
>>> +    uint16_t nl_family;     /* AF_NETLINK */
>>> +    int16_t __pad;
>>
>> netlink.h uses an unsigned type here.
> 
> This is padding, do you prefer char pad[2]?

kernel has:

struct sockaddr_nl {
        __kernel_sa_family_t    nl_family;      /* AF_NETLINK   */
        unsigned short  nl_pad;         /* zero         */
        __u32           nl_pid;         /* port ID      */
        __u32           nl_groups;      /* multicast groups mask */
};

so I prefer "uint16_t", but as you say, it is padding, so sign is
meaningless, if you prefer you can keep int16_t.

Now, it would be good to use target_sockaddr_nl in
host_to_target_sockaddr() as we define it.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 11/11] linux-user/syscall: simplify recvfrom()

[Laurent Vivier]

Le 24/01/2018 à 14:01, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  linux-user/syscall.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index b6b9beca5b..e082af2911 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -4023,8 +4023,8 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
>                              abi_ulong target_addr,
>                              abi_ulong target_addrlen)
>  {
> -    socklen_t addrlen;
> -    void *addr;
> +    socklen_t addrlen = 0;
> +    void *addr = NULL;
>      void *host_msg;
>      abi_long ret;
>  
> @@ -4046,12 +4046,9 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
>          }
>  
>          addr = alloca(addrlen);
> -        ret = get_errno(safe_recvfrom(fd, host_msg, len, flags,
> -                                      addr, &addrlen));
> -    } else {
> -        addr = NULL; /* To keep compiler quiet.  */
> -        ret = get_errno(safe_recvfrom(fd, host_msg, len, flags, NULL, 0));
>      }
> +    ret = get_errno(safe_recvfrom(fd, host_msg, len, flags, addr, &addrlen));
> +
>      if (!is_error(ret)) {
>          if (fd_trans_host_to_target_data(fd)) {
>              ret = fd_trans_host_to_target_data(fd)(host_msg, ret);
> 

I think it would be better to pass a NULL pointer to the function for
addrlen if addr is NULL.

In man page we have:

    If the caller is not interested in the source address, src_addr and
    addrlen should be specified as NULL.

Something like:

...
-    socklen_t addrlen;
-    void *addr;
+    socklen_t *addrlen = NULL;
+    void *addr = NULL;
+    socklen_t target_len;
...
-        addr = alloca(addrlen);
-        ret = get_errno(safe_recvfrom(fd, host_msg, len, flags,
-                                      addr, &addrlen));
-    } else {
-        addr = NULL; /* To keep compiler quiet.  */
-        ret = get_errno(safe_recvfrom(fd, host_msg, len, flags, NULL, 0));
+        addr = alloca(target_len);
+        addrlen = &target_len
     }
+    ret = get_errno(safe_recvfrom(fd, host_msg, len, flags, addr,
addrlen));
...


Thanks,
Laurent

Re: [Qemu-devel] [PATCH 10/11] linux-user/syscall: verify recvfrom(addr) is user-writable

[Laurent Vivier]

Le 24/01/2018 à 14:01, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  linux-user/syscall.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 11c9116c4a..b6b9beca5b 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -4040,6 +4040,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));
> 

Even if host_to_target_sockaddr() will do the check before copying data
to the target, I think it is good to check this before reading the data, so:

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

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 02/11] linux-user/strace: improve sendto() output

[Laurent Vivier]

Le 24/01/2018 à 14:01, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.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 7eb5e2ab48..e7272f4ede 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -1922,6 +1922,22 @@ print_socketcall(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(TARGET_ABI_FMT_ld, arg0, 0);

Other strace functions (accept(), fcntl(), ...) use "%d" for fd.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 09/11] linux-user/strace: improve capget()/capset() output

[Laurent Vivier]

Le 24/01/2018 à 14:01, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  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 f90c0e8ee4..f09234345f 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 },
> 

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

Re: [Qemu-devel] [PATCH 08/11] linux-user/strace: improve gettimeofday() output

[Laurent Vivier]

Le 24/01/2018 à 14:01, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.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 4b8ab6bcfb..b7c4cfae58 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -1526,6 +1526,19 @@ print_futimesat(const struct syscallname *name,
>  }
>  #endif
>  
> +#ifdef TARGET_NR_gettimeofday
> +static void
> +print_gettimeofday(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_pointer(arg0, 0);
> +    print_pointer(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 958d10d48f..f90c0e8ee4 100644
> --- a/linux-user/strace.list
> +++ b/linux-user/strace.list
> @@ -384,7 +384,7 @@
>  { TARGET_NR_gettid, "gettid" , "%s()", NULL, NULL },
>  #endif
>  #ifdef TARGET_NR_gettimeofday
> -{ TARGET_NR_gettimeofday, "gettimeofday" , NULL, NULL, NULL },
> +{ TARGET_NR_gettimeofday, "gettimeofday" , NULL, print_gettimeofday, NULL },
>  #endif
>  #ifdef TARGET_NR_getuid
>  { TARGET_NR_getuid, "getuid" , "%s()", NULL, NULL },
> 

There is a print_timeval(), and you could add a print_timezone(), and
update the result() function pointer to call them.

And once it is done, you can also display them for settimeofday().

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 07/11] linux-user/strace: improve bind() output

[Laurent Vivier]

Le 24/01/2018 à 14:01, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.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 70ecb1fc98..4b8ab6bcfb 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -1935,6 +1935,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(TARGET_ABI_FMT_ld, arg0, 0);

Other functions use "%d" for fd.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 06/11] linux-user/strace: improve recvmsg() output

[Laurent Vivier]

Le 24/01/2018 à 14:01, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  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 947ff04eab..ae6dc8fecf 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 },
> 

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

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

[Laurent Vivier]

Le 24/01/2018 à 14:01, Philippe Mathieu-Daudé a écrit :
> since this argument differs between sendto()/recvfrom()
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  linux-user/strace.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index e7272f4ede..9726d9b378 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -335,12 +335,15 @@ static void print_siginfo(const target_siginfo_t *tinfo)
>  }
>  
>  static void
> -print_sockaddr(abi_ulong addr, abi_long addrlen)
> +print_sockaddr_ptr(abi_ulong addr, abi_long addrlen, bool addrlen_ptr)
>  {
>      struct target_sockaddr *sa;
>      int i;
>      int sa_family;
>  
> +    if (addrlen_ptr) {
> +        get_user_ual(addrlen, addrlen);
> +    }
>      sa = lock_user(VERIFY_READ, addr, addrlen, 1);
>      if (sa) {
>          sa_family = tswap16(sa->sa_family);
> @@ -417,7 +420,17 @@ 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);
> +    if (addrlen_ptr) {
> +        gemu_log(", ["TARGET_ABI_FMT_ld"]", addrlen);
> +    } else {
> +        gemu_log(", "TARGET_ABI_FMT_ld, addrlen);
> +    }
> +}
> +
> +static void
> +print_sockaddr(abi_ulong addr, abi_long addrlen)
> +{
> +    print_sockaddr_ptr(addr, addrlen, false);
>  }
>  
>  static void
> 

Why not only something like:

static void
print_sockaddr_ptr(abi_ulong addr, abi_long addrlen_ptr)
{
    abi_ulong addrlen;

    get_user_ual(addrlen, addrlen_ptr);
    print_sockaddr(addr, addrlen);
}

?

You should also check addrlen_ptr != NULL (it's allowed with recvfrom()).

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 08/11] linux-user/strace: improve gettimeofday() output

[Philippe Mathieu-Daudé]

Hi Laurent,

On 02/17/2018 02:21 PM, Laurent Vivier wrote:
> Le 24/01/2018 à 14:01, Philippe Mathieu-Daudé a écrit :
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.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 4b8ab6bcfb..b7c4cfae58 100644
>> --- a/linux-user/strace.c
>> +++ b/linux-user/strace.c
>> @@ -1526,6 +1526,19 @@ print_futimesat(const struct syscallname *name,
>>  }
>>  #endif
>>  
>> +#ifdef TARGET_NR_gettimeofday
>> +static void
>> +print_gettimeofday(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_pointer(arg0, 0);
>> +    print_pointer(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 958d10d48f..f90c0e8ee4 100644
>> --- a/linux-user/strace.list
>> +++ b/linux-user/strace.list
>> @@ -384,7 +384,7 @@
>>  { TARGET_NR_gettid, "gettid" , "%s()", NULL, NULL },
>>  #endif
>>  #ifdef TARGET_NR_gettimeofday
>> -{ TARGET_NR_gettimeofday, "gettimeofday" , NULL, NULL, NULL },
>> +{ TARGET_NR_gettimeofday, "gettimeofday" , NULL, print_gettimeofday, NULL },
>>  #endif
>>  #ifdef TARGET_NR_getuid
>>  { TARGET_NR_getuid, "getuid" , "%s()", NULL, NULL },
>>
> 
> There is a print_timeval(), and you could add a print_timezone(), and
> update the result() function pointer to call them.

The result prototype is:

  void (*result)(const struct syscallname *, abi_long);

So we don't have access to the address of the struct timeval argument as
the call() function.

> 
> And once it is done, you can also display them for settimeofday().

OK.

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

[Philippe Mathieu-Daudé]

Hi Laurent,

On 02/17/2018 03:10 PM, Laurent Vivier wrote:
> Le 24/01/2018 à 14:01, Philippe Mathieu-Daudé a écrit :
>> since this argument differs between sendto()/recvfrom()
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  linux-user/strace.c | 17 +++++++++++++++--
>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/linux-user/strace.c b/linux-user/strace.c
>> index e7272f4ede..9726d9b378 100644
>> --- a/linux-user/strace.c
>> +++ b/linux-user/strace.c
>> @@ -335,12 +335,15 @@ static void print_siginfo(const target_siginfo_t *tinfo)
>>  }
>>  
>>  static void
>> -print_sockaddr(abi_ulong addr, abi_long addrlen)
>> +print_sockaddr_ptr(abi_ulong addr, abi_long addrlen, bool addrlen_ptr)
>>  {
>>      struct target_sockaddr *sa;
>>      int i;
>>      int sa_family;
>>  
>> +    if (addrlen_ptr) {
>> +        get_user_ual(addrlen, addrlen);
>> +    }
>>      sa = lock_user(VERIFY_READ, addr, addrlen, 1);
>>      if (sa) {
>>          sa_family = tswap16(sa->sa_family);
>> @@ -417,7 +420,17 @@ 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);
>> +    if (addrlen_ptr) {
>> +        gemu_log(", ["TARGET_ABI_FMT_ld"]", addrlen);
>> +    } else {
>> +        gemu_log(", "TARGET_ABI_FMT_ld, addrlen);
>> +    }
>> +}
>> +
>> +static void
>> +print_sockaddr(abi_ulong addr, abi_long addrlen)
>> +{
>> +    print_sockaddr_ptr(addr, addrlen, false);
>>  }
>>  
>>  static void
>>
> 
> Why not only something like:
> 
> static void
> print_sockaddr_ptr(abi_ulong addr, abi_long addrlen_ptr)
> {
>     abi_ulong addrlen;
> 
>     get_user_ual(addrlen, addrlen_ptr);
>     print_sockaddr(addr, addrlen);
> }
> 
> ?

Because we loose the [] around addrlen.

> You should also check addrlen_ptr != NULL (it's allowed with recvfrom()).

OK.

> 
> Thanks,
> Laurent
>