qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v12 0/5]
@ 2019-06-19 14:17 Aleksandar Markovic
  2019-06-19 14:17 ` [Qemu-devel] [PATCH v12 1/5] linux-user: Add support for setsockopt() options IPV6_<ADD|DROP>_MEMBERSHIP Aleksandar Markovic
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Aleksandar Markovic @ 2019-06-19 14:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent, amarkovic

From: Aleksandar Markovic <amarkovic@wavecomp.com>

This is a collection of misc patches for Linux user that I recently
accumulated from variuous sources. All of them originate from problems
observed on mips target. However, most of these changes actually
affect and fix linux-user problems on multiple targets.

There are three checkpatch warninhs and two checkpatch errors that
sould be ignored in the given circumstances.

v11->v12:

  - used the preffered Laurent's email address for linux-user
  - added a note on checkpetch warnings and errors in the commit letter
 

v10->v11:

  - patch on <ADD|DROP>_MEMEBERSHIP basically reverted to its first
    version
  - corrected patch on statx()
  - added patch on strace support for statx()
  - added patch on fixing flock structure for MIPS O64 ABI

v9->v10:

  - improved commit messages for patches 2 and 3

v8->v9:

  - fixed build error on some systems related to SOL_ALG

v7->v8:

  - added a patch on setsockopt() option SOL_ALG

v6->v7:

  - fixed a build error for older kernels related to the patch on
    setsockopt() options
  - removed four patches that on the meantime got accepted into the
    main source tree

v5->v6:

  - fixed a mistake in patch #4
  - improved commit messages in patches #4 and #6

v4->v5:

  - added the patch on statx() support
  - improved the patch on IPV6_<ADD|DROP>_MEMBERSHIP to take into
    account the possibility of different names for a field
  - minor corrections in commit messages

v3->v4:

  - improved commit messages (fixed some typos, improved relevance)

v2->v3:

  - updated and improved commit messages
  - added IPV6_DROP_MEMBERSHIP support to the patch on setsockopt()'s
    option

v1->v2:

  - added the patch on setsockopt()'s option IPV6_ADD_MEMBERSHIP
  - improved the commit messages

Aleksandar Markovic (1):
  linux-user: Fix flock structure for MIPS O64 ABI

Aleksandar Rikalo (1):
  linux-user: Add support for translation of statx() syscall

Jim Wilson (1):
  linux-user: Add support for strace for statx() syscall

Neng Chen (1):
  linux-user: Add support for setsockopt() options
    IPV6_<ADD|DROP>_MEMBERSHIP

Yunqiang Su (1):
  linux-user: Add support for setsockopt() option SOL_ALG

 linux-user/generic/fcntl.h     |   2 +-
 linux-user/mips/target_fcntl.h |   4 +
 linux-user/strace.c            |  86 +++++++++++++++++++
 linux-user/strace.list         |   3 +
 linux-user/syscall.c           | 186 ++++++++++++++++++++++++++++++++++++++++-
 linux-user/syscall_defs.h      |  37 ++++++++
 6 files changed, 316 insertions(+), 2 deletions(-)

-- 
2.7.4



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

* [Qemu-devel] [PATCH v12 1/5] linux-user: Add support for setsockopt() options IPV6_<ADD|DROP>_MEMBERSHIP
  2019-06-19 14:17 [Qemu-devel] [PATCH v12 0/5] Aleksandar Markovic
@ 2019-06-19 14:17 ` Aleksandar Markovic
  2019-06-19 16:08   ` Laurent Vivier
  2019-06-24 21:04   ` Laurent Vivier
  2019-06-19 14:17 ` [Qemu-devel] [PATCH v12 2/5] linux-user: Add support for setsockopt() option SOL_ALG Aleksandar Markovic
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 17+ messages in thread
From: Aleksandar Markovic @ 2019-06-19 14:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Neng Chen, laurent, amarkovic

From: Neng Chen <nchen@wavecomp.com>

Add support for the option IPV6_<ADD|DROP>_MEMBERSHIP of the syscall
setsockopt(). This option controls membership in multicast groups.
Argument is a pointer to a struct ipv6_mreq.

The glibc <netinet/in.h> header defines the ipv6_mreq structure,
which includes the following members:

  struct in6_addr  ipv6mr_multiaddr;
  unsigned int     ipv6mr_interface;

Whereas the kernel in its <linux/in6.h> header defines following
members of the same structure:

  struct in6_addr  ipv6mr_multiaddr;
  int              ipv6mr_ifindex;

POSIX defines ipv6mr_interface [1].

__UAPI_DEF_IVP6_MREQ appears in kernel headers with v3.12:

  cfd280c91253 net: sync some IP headers with glibc

Without __UAPI_DEF_IVP6_MREQ, kernel defines ipv6mr_ifindex, and
this is explained in cfd280c91253:

  "If you include the kernel headers first you get those,
  and if you include the glibc headers first you get those,
  and the following patch arranges a coordination and
  synchronization between the two."

So before 3.12, a program can't include both <netinet/in.h> and
<linux/in6.h>.

In linux-user/syscall.c, we only include <netinet/in.h> (glibc) and
not <linux/in6.h> (kernel headers), so ipv6mr_interface is the one
to use.

[1] http://pubs.opengroup.org/onlinepubs/009695399/basedefs/netinet/in.h.html

Signed-off-by: Neng Chen <nchen@wavecomp.com>
Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
---
 linux-user/syscall.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index b187c12..f267ad0 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1920,6 +1920,25 @@ static abi_long do_setsockopt(int sockfd, int level, int optname,
                                        &pki, sizeof(pki)));
             break;
         }
+        case IPV6_ADD_MEMBERSHIP:
+        case IPV6_DROP_MEMBERSHIP:
+        {
+            struct ipv6_mreq ipv6mreq;
+
+            if (optlen < sizeof(ipv6mreq)) {
+                return -TARGET_EINVAL;
+            }
+
+            if (copy_from_user(&ipv6mreq, optval_addr, sizeof(ipv6mreq))) {
+                return -TARGET_EFAULT;
+            }
+
+            ipv6mreq.ipv6mr_interface = tswap32(ipv6mreq.ipv6mr_interface);
+
+            ret = get_errno(setsockopt(sockfd, level, optname,
+                                       &ipv6mreq, sizeof(ipv6mreq)));
+            break;
+        }
         default:
             goto unimplemented;
         }
-- 
2.7.4



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

* [Qemu-devel] [PATCH v12 2/5] linux-user: Add support for setsockopt() option SOL_ALG
  2019-06-19 14:17 [Qemu-devel] [PATCH v12 0/5] Aleksandar Markovic
  2019-06-19 14:17 ` [Qemu-devel] [PATCH v12 1/5] linux-user: Add support for setsockopt() options IPV6_<ADD|DROP>_MEMBERSHIP Aleksandar Markovic
@ 2019-06-19 14:17 ` Aleksandar Markovic
  2019-06-24 21:01   ` Laurent Vivier
  2019-06-19 14:17 ` [Qemu-devel] [PATCH v12 3/5] linux-user: Add support for translation of statx() syscall Aleksandar Markovic
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Aleksandar Markovic @ 2019-06-19 14:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Yunqiang Su, laurent, amarkovic

From: Yunqiang Su <ysu@wavecomp.com>

Add support for options SOL_ALG of the syscall setsockopt(). This
option is used in relation to Linux kernel Crypto API, and allows
a user to set additional information for the cipher operation via
syscall setsockopt(). The field "optname" must be one of the
following:

  - ALG_SET_KEY – seting the key
  - ALG_SET_AEAD_AUTHSIZE – set the authentication tag size

SOL_ALG is relatively newer setsockopt() option. Therefore, the
code that handles SOL_ALG is enclosed in "ifdef" so that the build
does not fail for older kernels that do not contain support for
SOL_ALG. "ifdef" also contains check if ALG_SET_KEY and
ALG_SET_AEAD_AUTHSIZE are defined.

Signed-off-by: Yunqiang Su <ysu@wavecomp.com>
Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/syscall.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f267ad0..d116287 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -102,6 +102,7 @@
 #include <linux/blkpg.h>
 #include <netpacket/packet.h>
 #include <linux/netlink.h>
+#include <linux/if_alg.h>
 #include "linux_loop.h"
 #include "uname.h"
 
@@ -1989,6 +1990,36 @@ static abi_long do_setsockopt(int sockfd, int level, int optname,
             goto unimplemented;
         }
         break;
+#if defined(SOL_ALG) && defined(ALG_SET_KEY) && defined(ALG_SET_AEAD_AUTHSIZE)
+    case SOL_ALG:
+        switch (optname) {
+        case ALG_SET_KEY:
+        {
+            char *alg_key = g_malloc(optlen);
+
+            if (!alg_key) {
+                return -TARGET_ENOMEM;
+            }
+            if (copy_from_user(alg_key, optval_addr, optlen)) {
+                g_free(alg_key);
+                return -TARGET_EFAULT;
+            }
+            ret = get_errno(setsockopt(sockfd, level, optname,
+                                       alg_key, optlen));
+            g_free(alg_key);
+            break;
+        }
+        case ALG_SET_AEAD_AUTHSIZE:
+        {
+            ret = get_errno(setsockopt(sockfd, level, optname,
+                                       NULL, optlen));
+            break;
+        }
+        default:
+            goto unimplemented;
+        }
+        break;
+#endif
     case TARGET_SOL_SOCKET:
         switch (optname) {
         case TARGET_SO_RCVTIMEO:
-- 
2.7.4



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

* [Qemu-devel] [PATCH v12 3/5] linux-user: Add support for translation of statx() syscall
  2019-06-19 14:17 [Qemu-devel] [PATCH v12 0/5] Aleksandar Markovic
  2019-06-19 14:17 ` [Qemu-devel] [PATCH v12 1/5] linux-user: Add support for setsockopt() options IPV6_<ADD|DROP>_MEMBERSHIP Aleksandar Markovic
  2019-06-19 14:17 ` [Qemu-devel] [PATCH v12 2/5] linux-user: Add support for setsockopt() option SOL_ALG Aleksandar Markovic
@ 2019-06-19 14:17 ` Aleksandar Markovic
  2019-06-19 16:11   ` Laurent Vivier
  2019-06-19 14:17 ` [Qemu-devel] [PATCH v12 4/5] linux-user: Add support for strace for " Aleksandar Markovic
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Aleksandar Markovic @ 2019-06-19 14:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Aleksandar Rikalo, laurent, amarkovic

From: Aleksandar Rikalo <arikalo@wavecomp.com>

Implement support for translation of system call statx().

The implementation is based on "best effort" approach: if host is
capable of executing statx(), host statx() is used. If not, the
implementation includes invoking other (more mature) system calls
(from the same 'stat' family) on the host side to achieve as close
as possible functionality.

Support for statx() in kernel and glibc was, however, introduced
at different points of time (the difference is more than a year):

  - kernel: Linux 4.11 (30 April 2017)
  - glibc: glibc 2.28 (1 Aug 2018)

In this patch, the availability of statx() support is established
via __NR_statx (if it is defined, statx() is considered available).
This coincedes with statx() introduction in kernel.

However, the structure statx definition may not be available for hosts
with glibc older than 2.28 (it is, by design, to be defined in one of
glibc headers), even though the full statx() functionality may be
supported in kernel, if the kernel is not older than 4.11. Hence,
a structure "target_statx" is defined in this patch, to remove that
dependency on glibc headers, and to use statx() functionality as soon
as the host kernel is capable of supporting it. Such structure statx
definition is used for both target and host structures statx (of
course, this doesn't mean the endian arrangement is the same on
target and host, and endian conversion is done in all necessary
cases).

Signed-off-by: Aleksandar Rikalo <arikalo@wavecomp.com>
Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
---
 linux-user/syscall.c      | 136 +++++++++++++++++++++++++++++++++++++++++++++-
 linux-user/syscall_defs.h |  37 +++++++++++++
 2 files changed, 172 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index d116287..e68a36c 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -43,6 +43,7 @@
 #include <sys/times.h>
 #include <sys/shm.h>
 #include <sys/sem.h>
+#include <sys/stat.h>
 #include <sys/statfs.h>
 #include <utime.h>
 #include <sys/sysinfo.h>
@@ -316,6 +317,14 @@ _syscall5(int, kcmp, pid_t, pid1, pid_t, pid2, int, type,
           unsigned long, idx1, unsigned long, idx2)
 #endif
 
+/*
+ * It is assumed that struct statx is architecture independent.
+ */
+#if defined(TARGET_NR_statx) && defined(__NR_statx)
+_syscall5(int, statx, int, dirfd, const char *, pathname, int, flags,
+          unsigned int, mask, struct target_statx *, statxbuf)
+#endif
+
 static bitmask_transtbl fcntl_flags_tbl[] = {
   { TARGET_O_ACCMODE,   TARGET_O_WRONLY,    O_ACCMODE,   O_WRONLY,    },
   { TARGET_O_ACCMODE,   TARGET_O_RDWR,      O_ACCMODE,   O_RDWR,      },
@@ -6517,6 +6526,48 @@ static inline abi_long host_to_target_stat64(void *cpu_env,
 }
 #endif
 
+#if defined(TARGET_NR_statx) && defined(__NR_statx)
+static inline abi_long host_to_target_statx(struct target_statx *host_stx,
+                                            abi_ulong target_addr)
+{
+    struct target_statx *target_stx;
+
+    if (!lock_user_struct(VERIFY_WRITE, target_stx, target_addr,  0)) {
+        return -TARGET_EFAULT;
+    }
+    memset(target_stx, 0, sizeof(*target_stx));
+
+    __put_user(host_stx->stx_mask, &target_stx->stx_mask);
+    __put_user(host_stx->stx_blksize, &target_stx->stx_blksize);
+    __put_user(host_stx->stx_attributes, &target_stx->stx_attributes);
+    __put_user(host_stx->stx_nlink, &target_stx->stx_nlink);
+    __put_user(host_stx->stx_uid, &target_stx->stx_uid);
+    __put_user(host_stx->stx_gid, &target_stx->stx_gid);
+    __put_user(host_stx->stx_mode, &target_stx->stx_mode);
+    __put_user(host_stx->stx_ino, &target_stx->stx_ino);
+    __put_user(host_stx->stx_size, &target_stx->stx_size);
+    __put_user(host_stx->stx_blocks, &target_stx->stx_blocks);
+    __put_user(host_stx->stx_attributes_mask, &target_stx->stx_attributes_mask);
+    __put_user(host_stx->stx_atime.tv_sec, &target_stx->stx_atime.tv_sec);
+    __put_user(host_stx->stx_atime.tv_nsec, &target_stx->stx_atime.tv_nsec);
+    __put_user(host_stx->stx_btime.tv_sec, &target_stx->stx_atime.tv_sec);
+    __put_user(host_stx->stx_btime.tv_nsec, &target_stx->stx_atime.tv_nsec);
+    __put_user(host_stx->stx_ctime.tv_sec, &target_stx->stx_atime.tv_sec);
+    __put_user(host_stx->stx_ctime.tv_nsec, &target_stx->stx_atime.tv_nsec);
+    __put_user(host_stx->stx_mtime.tv_sec, &target_stx->stx_atime.tv_sec);
+    __put_user(host_stx->stx_mtime.tv_nsec, &target_stx->stx_atime.tv_nsec);
+    __put_user(host_stx->stx_rdev_major, &target_stx->stx_rdev_major);
+    __put_user(host_stx->stx_rdev_minor, &target_stx->stx_rdev_minor);
+    __put_user(host_stx->stx_dev_major, &target_stx->stx_dev_major);
+    __put_user(host_stx->stx_dev_minor, &target_stx->stx_dev_minor);
+
+    unlock_user_struct(target_stx, target_addr, 1);
+
+    return 0;
+}
+#endif
+
+
 /* ??? Using host futex calls even when target atomic operations
    are not really atomic probably breaks things.  However implementing
    futexes locally would make futexes shared between multiple processes
@@ -7095,7 +7146,8 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
     abi_long ret;
 #if defined(TARGET_NR_stat) || defined(TARGET_NR_stat64) \
     || defined(TARGET_NR_lstat) || defined(TARGET_NR_lstat64) \
-    || defined(TARGET_NR_fstat) || defined(TARGET_NR_fstat64)
+    || defined(TARGET_NR_fstat) || defined(TARGET_NR_fstat64) \
+    || defined(TARGET_NR_statx)
     struct stat st;
 #endif
 #if defined(TARGET_NR_statfs) || defined(TARGET_NR_statfs64) \
@@ -10173,6 +10225,88 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
             ret = host_to_target_stat64(cpu_env, arg3, &st);
         return ret;
 #endif
+#if defined(TARGET_NR_statx)
+    case TARGET_NR_statx:
+        {
+            struct target_statx *target_stx;
+            int dirfd = arg1;
+            int flags = arg3;
+
+            p = lock_user_string(arg2);
+            if (p == NULL) {
+                return -TARGET_EFAULT;
+            }
+#if defined(__NR_statx)
+            {
+                /*
+                 * It is assumed that struct statx is architecture independent.
+                 */
+                struct target_statx host_stx;
+                int mask = arg4;
+
+                ret = get_errno(statx(dirfd, p, flags, mask, &host_stx));
+                if (!is_error(ret)) {
+                    if (host_to_target_statx(&host_stx, arg5) != 0) {
+                        unlock_user(p, arg2, 0);
+                        return -TARGET_EFAULT;
+                    }
+                }
+
+                if (ret != -TARGET_ENOSYS) {
+                    unlock_user(p, arg2, 0);
+                    return ret;
+                }
+            }
+#endif
+            if (*((char *)p) == 0) {
+                /*
+                 * By file descriptor
+                 */
+                if (flags & AT_EMPTY_PATH) {
+                    unlock_user(p, arg2, 0);
+                    return -TARGET_ENOENT;
+                }
+                ret = get_errno(fstat(dirfd, &st));
+            } else if (*((char *)p) == '/') {
+                /*
+                 * By absolute pathname
+                 */
+                ret = get_errno(stat(path(p), &st));
+            } else {
+                /*
+                 * By pathname relative to the current working directory
+                 * (if 'dirfd' is AT_FDCWD) or relative to the directory
+                 * referred to by the file descriptor 'dirfd'.
+                 */
+                 ret = get_errno(fstatat(dirfd, path(p), &st, flags));
+            }
+            unlock_user(p, arg2, 0);
+
+            if (!is_error(ret)) {
+                if (!lock_user_struct(VERIFY_WRITE, target_stx, arg5, 0)) {
+                    return -TARGET_EFAULT;
+                }
+                memset(target_stx, 0, sizeof(*target_stx));
+                __put_user(major(st.st_dev), &target_stx->stx_dev_major);
+                __put_user(minor(st.st_dev), &target_stx->stx_dev_minor);
+                __put_user(st.st_ino, &target_stx->stx_ino);
+                __put_user(st.st_mode, &target_stx->stx_mode);
+                __put_user(st.st_uid, &target_stx->stx_uid);
+                __put_user(st.st_gid, &target_stx->stx_gid);
+                __put_user(st.st_nlink, &target_stx->stx_nlink);
+                __put_user(major(st.st_rdev), &target_stx->stx_rdev_major);
+                __put_user(minor(st.st_rdev), &target_stx->stx_rdev_minor);
+                __put_user(st.st_size, &target_stx->stx_size);
+                __put_user(st.st_blksize, &target_stx->stx_blksize);
+                __put_user(st.st_blocks, &target_stx->stx_blocks);
+                __put_user(st.st_atime, &target_stx->stx_atime.tv_sec);
+                __put_user(st.st_mtime, &target_stx->stx_mtime.tv_sec);
+                __put_user(st.st_ctime, &target_stx->stx_ctime.tv_sec);
+                unlock_user_struct(target_stx, arg5, 1);
+            }
+        }
+        return ret;
+#endif
 #ifdef TARGET_NR_lchown
     case TARGET_NR_lchown:
         if (!(p = lock_user_string(arg1)))
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 7f141f6..170c4dd 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -2536,4 +2536,41 @@ struct target_user_cap_data {
 /* Return size of the log buffer */
 #define TARGET_SYSLOG_ACTION_SIZE_BUFFER   10
 
+struct target_statx_timestamp {
+   int64_t tv_sec;
+   uint32_t tv_nsec;
+   int32_t __reserved;
+};
+
+struct target_statx {
+   /* 0x00 */
+   uint32_t stx_mask;       /* What results were written [uncond] */
+   uint32_t stx_blksize;    /* Preferred general I/O size [uncond] */
+   uint64_t stx_attributes; /* Flags conveying information about the file */
+   /* 0x10 */
+   uint32_t stx_nlink;      /* Number of hard links */
+   uint32_t stx_uid;        /* User ID of owner */
+   uint32_t stx_gid;        /* Group ID of owner */
+   uint16_t stx_mode;       /* File mode */
+   uint16_t __spare0[1];
+   /* 0x20 */
+   uint64_t stx_ino;        /* Inode number */
+   uint64_t stx_size;       /* File size */
+   uint64_t stx_blocks;     /* Number of 512-byte blocks allocated */
+   uint64_t stx_attributes_mask; /* Mask to show what is supported */
+   /* 0x40 */
+   struct target_statx_timestamp  stx_atime;  /* Last access time */
+   struct target_statx_timestamp  stx_btime;  /* File creation time */
+   struct target_statx_timestamp  stx_ctime;  /* Last attribute change time */
+   struct target_statx_timestamp  stx_mtime;  /* Last data modification time */
+   /* 0x80 */
+   uint32_t stx_rdev_major;   /* Device ID of special file [if bdev/cdev] */
+   uint32_t stx_rdev_minor;
+   uint32_t stx_dev_major; /* ID of device containing file [uncond] */
+   uint32_t stx_dev_minor;
+   /* 0x90 */
+   uint64_t __spare2[14];  /* Spare space for future expansion */
+   /* 0x100 */
+};
+
 #endif
-- 
2.7.4



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

* [Qemu-devel] [PATCH v12 4/5] linux-user: Add support for strace for statx() syscall
  2019-06-19 14:17 [Qemu-devel] [PATCH v12 0/5] Aleksandar Markovic
                   ` (2 preceding siblings ...)
  2019-06-19 14:17 ` [Qemu-devel] [PATCH v12 3/5] linux-user: Add support for translation of statx() syscall Aleksandar Markovic
@ 2019-06-19 14:17 ` Aleksandar Markovic
  2019-06-19 14:17 ` [Qemu-devel] [PATCH v12 5/5] linux-user: Fix flock structure for MIPS O64 ABI Aleksandar Markovic
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Aleksandar Markovic @ 2019-06-19 14:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent, amarkovic, Jim Wilson

From: Jim Wilson <jimw@sifive.com>

All of the flags need to be conditional as old systems don't have statx
support.  Otherwise it works the same as other stat family syscalls.
This requires the pending patch to add statx support.

Tested on Ubuntu 16.04 (no host statx) and Ubuntu 19.04 (with host statx)
using a riscv32-linux toolchain.

Signed-off-by: Jim Wilson <jimw@sifive.com>
Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/strace.c    | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++
 linux-user/strace.list |  3 ++
 2 files changed, 89 insertions(+)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 6f72a74..c80e93b 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -976,6 +976,76 @@ UNUSED static struct flags msg_flags[] = {
     FLAG_END,
 };
 
+UNUSED static struct flags statx_flags[] = {
+#ifdef AT_EMPTY_PATH
+    FLAG_GENERIC(AT_EMPTY_PATH),
+#endif
+#ifdef AT_NO_AUTOMOUNT
+    FLAG_GENERIC(AT_NO_AUTOMOUNT),
+#endif
+#ifdef AT_SYMLINK_NOFOLLOW
+    FLAG_GENERIC(AT_SYMLINK_NOFOLLOW),
+#endif
+#ifdef AT_STATX_SYNC_AS_STAT
+    FLAG_GENERIC(AT_STATX_SYNC_AS_STAT),
+#endif
+#ifdef AT_STATX_FORCE_SYNC
+    FLAG_GENERIC(AT_STATX_FORCE_SYNC),
+#endif
+#ifdef AT_STATX_DONT_SYNC
+    FLAG_GENERIC(AT_STATX_DONT_SYNC),
+#endif
+    FLAG_END,
+};
+
+UNUSED static struct flags statx_mask[] = {
+/* This must come first, because it includes everything.  */
+#ifdef STATX_ALL
+    FLAG_GENERIC(STATX_ALL),
+#endif
+/* This must come second; it includes everything except STATX_BTIME.  */
+#ifdef STATX_BASIC_STATS
+    FLAG_GENERIC(STATX_BASIC_STATS),
+#endif
+#ifdef STATX_TYPE
+    FLAG_GENERIC(STATX_TYPE),
+#endif
+#ifdef STATX_MODE
+    FLAG_GENERIC(STATX_MODE),
+#endif
+#ifdef STATX_NLINK
+    FLAG_GENERIC(STATX_NLINK),
+#endif
+#ifdef STATX_UID
+    FLAG_GENERIC(STATX_UID),
+#endif
+#ifdef STATX_GID
+    FLAG_GENERIC(STATX_GID),
+#endif
+#ifdef STATX_ATIME
+    FLAG_GENERIC(STATX_ATIME),
+#endif
+#ifdef STATX_MTIME
+    FLAG_GENERIC(STATX_MTIME),
+#endif
+#ifdef STATX_CTIME
+    FLAG_GENERIC(STATX_CTIME),
+#endif
+#ifdef STATX_INO
+    FLAG_GENERIC(STATX_INO),
+#endif
+#ifdef STATX_SIZE
+    FLAG_GENERIC(STATX_SIZE),
+#endif
+#ifdef STATX_BLOCKS
+    FLAG_GENERIC(STATX_BLOCKS),
+#endif
+#ifdef STATX_BTIME
+    FLAG_GENERIC(STATX_BTIME),
+#endif
+    FLAG_END,
+};
+
 /*
  * print_xxx utility functions.  These are used to print syscall
  * parameters in certain format.  All of these have parameter
@@ -2611,6 +2681,22 @@ print_tgkill(const struct syscallname *name,
 }
 #endif
 
+#ifdef TARGET_NR_statx
+static void
+print_statx(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_at_dirfd(arg0, 0);
+    print_string(arg1, 0);
+    print_flags(statx_flags, arg2, 0);
+    print_flags(statx_mask, arg3, 0);
+    print_pointer(arg4, 1);
+    print_syscall_epilogue(name);
+}
+#endif
+
 /*
  * An array of all of the syscalls we know about
  */
diff --git a/linux-user/strace.list b/linux-user/strace.list
index db21ce4..63a9466 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -1650,3 +1650,6 @@
 #ifdef TARGET_NR_atomic_barrier
 { TARGET_NR_atomic_barrier, "atomic_barrier", NULL, NULL, NULL },
 #endif
+#ifdef TARGET_NR_statx
+{ TARGET_NR_statx, "statx", NULL, print_statx, NULL },
+#endif
-- 
2.7.4



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

* [Qemu-devel] [PATCH v12 5/5] linux-user: Fix flock structure for MIPS O64 ABI
  2019-06-19 14:17 [Qemu-devel] [PATCH v12 0/5] Aleksandar Markovic
                   ` (3 preceding siblings ...)
  2019-06-19 14:17 ` [Qemu-devel] [PATCH v12 4/5] linux-user: Add support for strace for " Aleksandar Markovic
@ 2019-06-19 14:17 ` Aleksandar Markovic
  2019-06-19 16:21   ` Laurent Vivier
  2019-06-19 14:54 ` [Qemu-devel] [PATCH v12 0/5] no-reply
  2019-06-19 15:34 ` no-reply
  6 siblings, 1 reply; 17+ messages in thread
From: Aleksandar Markovic @ 2019-06-19 14:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent, amarkovic

From: Aleksandar Markovic <amarkovic@wavecomp.com>

Only MIPS O32 and N32 have special (different than other
architectures) definition of structure flock in kernel.

Bring flock definition for MIPS O64 ABI to the correct state.

Reported-by: Dragan Mladjenovic <dmladjenovic@wavecomp.com>
Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
---
 linux-user/generic/fcntl.h     | 2 +-
 linux-user/mips/target_fcntl.h | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/linux-user/generic/fcntl.h b/linux-user/generic/fcntl.h
index a775a49..1b48dde 100644
--- a/linux-user/generic/fcntl.h
+++ b/linux-user/generic/fcntl.h
@@ -129,7 +129,7 @@ struct target_flock {
     short l_whence;
     abi_long l_start;
     abi_long l_len;
-#if defined(TARGET_MIPS)
+#if defined(TARGET_MIPS) && (TARGET_ABI_BITS == 32)
     abi_long l_sysid;
 #endif
     int l_pid;
diff --git a/linux-user/mips/target_fcntl.h b/linux-user/mips/target_fcntl.h
index 000527c..795bba7 100644
--- a/linux-user/mips/target_fcntl.h
+++ b/linux-user/mips/target_fcntl.h
@@ -27,7 +27,11 @@
 #define TARGET_F_SETOWN        24       /*  for sockets. */
 #define TARGET_F_GETOWN        23       /*  for sockets. */
 
+#if (TARGET_ABI_BITS == 32)
 #define TARGET_ARCH_FLOCK_PAD abi_long pad[4];
+#else
+#define TARGET_ARCH_FLOCK_PAD
+#endif
 #define TARGET_ARCH_FLOCK64_PAD
 
 #define TARGET_F_GETLK64       33      /*  using 'struct flock64' */
-- 
2.7.4



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

* Re: [Qemu-devel] [PATCH v12 0/5]
  2019-06-19 14:17 [Qemu-devel] [PATCH v12 0/5] Aleksandar Markovic
                   ` (4 preceding siblings ...)
  2019-06-19 14:17 ` [Qemu-devel] [PATCH v12 5/5] linux-user: Fix flock structure for MIPS O64 ABI Aleksandar Markovic
@ 2019-06-19 14:54 ` no-reply
  2019-06-19 15:34 ` no-reply
  6 siblings, 0 replies; 17+ messages in thread
From: no-reply @ 2019-06-19 14:54 UTC (permalink / raw)
  To: aleksandar.markovic; +Cc: qemu-devel, amarkovic, laurent

Patchew URL: https://patchew.org/QEMU/1560953834-29584-1-git-send-email-aleksandar.markovic@rt-rk.com/



Hi,

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

Subject: [Qemu-devel] [PATCH v12 0/5]
Type: series
Message-id: 1560953834-29584-1-git-send-email-aleksandar.markovic@rt-rk.com

=== 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 ===

From https://github.com/patchew-project/qemu
 * [new tag]               patchew/1560953834-29584-1-git-send-email-aleksandar.markovic@rt-rk.com -> patchew/1560953834-29584-1-git-send-email-aleksandar.markovic@rt-rk.com
Switched to a new branch 'test'
da94588f68 linux-user: Fix flock structure for MIPS O64 ABI
61a9980aca linux-user: Add support for strace for statx() syscall
5058c9cfc7 linux-user: Add support for translation of statx() syscall
7b71d1b65a linux-user: Add support for setsockopt() option SOL_ALG
174903e7a9 linux-user: Add support for setsockopt() options IPV6_<ADD|DROP>_MEMBERSHIP

=== OUTPUT BEGIN ===
1/5 Checking commit 174903e7a97c (linux-user: Add support for setsockopt() options IPV6_<ADD|DROP>_MEMBERSHIP)
2/5 Checking commit 7b71d1b65a34 (linux-user: Add support for setsockopt() option SOL_ALG)
3/5 Checking commit 5058c9cfc7de (linux-user: Add support for translation of statx() syscall)
WARNING: architecture specific defines should be avoided
#60: FILE: linux-user/syscall.c:323:
+#if defined(TARGET_NR_statx) && defined(__NR_statx)

WARNING: architecture specific defines should be avoided
#72: FILE: linux-user/syscall.c:6529:
+#if defined(TARGET_NR_statx) && defined(__NR_statx)

WARNING: architecture specific defines should be avoided
#142: FILE: linux-user/syscall.c:10239:
+#if defined(__NR_statx)

total: 0 errors, 3 warnings, 207 lines checked

Patch 3/5 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/5 Checking commit 61a9980acaf8 (linux-user: Add support for strace for statx() syscall)
ERROR: storage class should be at the beginning of the declaration
#27: FILE: linux-user/strace.c:979:
+UNUSED static struct flags statx_flags[] = {

ERROR: storage class should be at the beginning of the declaration
#49: FILE: linux-user/strace.c:1001:
+UNUSED static struct flags statx_mask[] = {

total: 2 errors, 0 warnings, 104 lines checked

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

5/5 Checking commit da94588f687d (linux-user: Fix flock structure for MIPS O64 ABI)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/1560953834-29584-1-git-send-email-aleksandar.markovic@rt-rk.com/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] 17+ messages in thread

* Re: [Qemu-devel] [PATCH v12 0/5]
  2019-06-19 14:17 [Qemu-devel] [PATCH v12 0/5] Aleksandar Markovic
                   ` (5 preceding siblings ...)
  2019-06-19 14:54 ` [Qemu-devel] [PATCH v12 0/5] no-reply
@ 2019-06-19 15:34 ` no-reply
  6 siblings, 0 replies; 17+ messages in thread
From: no-reply @ 2019-06-19 15:34 UTC (permalink / raw)
  To: aleksandar.markovic; +Cc: qemu-devel, amarkovic, laurent

Patchew URL: https://patchew.org/QEMU/1560953834-29584-1-git-send-email-aleksandar.markovic@rt-rk.com/



Hi,

This series failed build test on s390x host. Please find the details below.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
set -e
CC=$HOME/bin/cc
INSTALL=$PWD/install
BUILD=$PWD/build
mkdir -p $BUILD $INSTALL
SRC=$PWD
cd $BUILD
$SRC/configure --cc=$CC --prefix=$INSTALL
make -j4
# XXX: we need reliable clean up
# make check -j4 V=1
make install

echo
echo "=== ENV ==="
env

echo
echo "=== PACKAGES ==="
rpm -qa
=== TEST SCRIPT END ===

  CC      mips64el-linux-user/linux-user/syscall.o
  CC      mips64-linux-user/accel/tcg/user-exec-stub.o
  CC      mips64-linux-user/linux-user/main.o
/var/tmp/patchew-tester-tmp-o2hme1xk/src/linux-user/syscall.c:324:16: error: conflicting types for ‘statx’
  324 | _syscall5(int, statx, int, dirfd, const char *, pathname, int, flags,
      |                ^~~~~
/var/tmp/patchew-tester-tmp-o2hme1xk/src/linux-user/syscall.c:215:13: note: in definition of macro ‘_syscall5’
---
  CC      microblaze-linux-user/disas.o
  CC      mips64-linux-user/linux-user/syscall.o
  CC      mips64-linux-user/linux-user/strace.o
/var/tmp/patchew-tester-tmp-o2hme1xk/src/linux-user/syscall.c:324:16: error: conflicting types for ‘statx’
  324 | _syscall5(int, statx, int, dirfd, const char *, pathname, int, flags,
      |                ^~~~~
/var/tmp/patchew-tester-tmp-o2hme1xk/src/linux-user/syscall.c:215:13: note: in definition of macro ‘_syscall5’


The full log is available at
http://patchew.org/logs/1560953834-29584-1-git-send-email-aleksandar.markovic@rt-rk.com/testing.s390x/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

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

* Re: [Qemu-devel] [PATCH v12 1/5] linux-user: Add support for setsockopt() options IPV6_<ADD|DROP>_MEMBERSHIP
  2019-06-19 14:17 ` [Qemu-devel] [PATCH v12 1/5] linux-user: Add support for setsockopt() options IPV6_<ADD|DROP>_MEMBERSHIP Aleksandar Markovic
@ 2019-06-19 16:08   ` Laurent Vivier
  2019-06-24 21:04   ` Laurent Vivier
  1 sibling, 0 replies; 17+ messages in thread
From: Laurent Vivier @ 2019-06-19 16:08 UTC (permalink / raw)
  To: Aleksandar Markovic, qemu-devel; +Cc: Neng Chen, amarkovic

Le 19/06/2019 à 16:17, Aleksandar Markovic a écrit :
> From: Neng Chen <nchen@wavecomp.com>
> 
> Add support for the option IPV6_<ADD|DROP>_MEMBERSHIP of the syscall
> setsockopt(). This option controls membership in multicast groups.
> Argument is a pointer to a struct ipv6_mreq.
> 
> The glibc <netinet/in.h> header defines the ipv6_mreq structure,
> which includes the following members:
> 
>   struct in6_addr  ipv6mr_multiaddr;
>   unsigned int     ipv6mr_interface;
> 
> Whereas the kernel in its <linux/in6.h> header defines following
> members of the same structure:
> 
>   struct in6_addr  ipv6mr_multiaddr;
>   int              ipv6mr_ifindex;
> 
> POSIX defines ipv6mr_interface [1].
> 
> __UAPI_DEF_IVP6_MREQ appears in kernel headers with v3.12:
> 
>   cfd280c91253 net: sync some IP headers with glibc
> 
> Without __UAPI_DEF_IVP6_MREQ, kernel defines ipv6mr_ifindex, and
> this is explained in cfd280c91253:
> 
>   "If you include the kernel headers first you get those,
>   and if you include the glibc headers first you get those,
>   and the following patch arranges a coordination and
>   synchronization between the two."
> 
> So before 3.12, a program can't include both <netinet/in.h> and
> <linux/in6.h>.
> 
> In linux-user/syscall.c, we only include <netinet/in.h> (glibc) and
> not <linux/in6.h> (kernel headers), so ipv6mr_interface is the one
> to use.
> 
> [1] http://pubs.opengroup.org/onlinepubs/009695399/basedefs/netinet/in.h.html
> 
> Signed-off-by: Neng Chen <nchen@wavecomp.com>
> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> ---
>  linux-user/syscall.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index b187c12..f267ad0 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1920,6 +1920,25 @@ static abi_long do_setsockopt(int sockfd, int level, int optname,
>                                         &pki, sizeof(pki)));
>              break;
>          }
> +        case IPV6_ADD_MEMBERSHIP:
> +        case IPV6_DROP_MEMBERSHIP:
> +        {
> +            struct ipv6_mreq ipv6mreq;
> +
> +            if (optlen < sizeof(ipv6mreq)) {
> +                return -TARGET_EINVAL;
> +            }
> +
> +            if (copy_from_user(&ipv6mreq, optval_addr, sizeof(ipv6mreq))) {
> +                return -TARGET_EFAULT;
> +            }
> +
> +            ipv6mreq.ipv6mr_interface = tswap32(ipv6mreq.ipv6mr_interface);
> +
> +            ret = get_errno(setsockopt(sockfd, level, optname,
> +                                       &ipv6mreq, sizeof(ipv6mreq)));
> +            break;
> +        }
>          default:
>              goto unimplemented;
>          }
> 

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


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

* Re: [Qemu-devel] [PATCH v12 3/5] linux-user: Add support for translation of statx() syscall
  2019-06-19 14:17 ` [Qemu-devel] [PATCH v12 3/5] linux-user: Add support for translation of statx() syscall Aleksandar Markovic
@ 2019-06-19 16:11   ` Laurent Vivier
  2019-06-27 13:18     ` Aleksandar Markovic
  0 siblings, 1 reply; 17+ messages in thread
From: Laurent Vivier @ 2019-06-19 16:11 UTC (permalink / raw)
  To: Aleksandar Markovic, qemu-devel; +Cc: Aleksandar Rikalo, amarkovic

Le 19/06/2019 à 16:17, Aleksandar Markovic a écrit :
> From: Aleksandar Rikalo <arikalo@wavecomp.com>
> 
> Implement support for translation of system call statx().
> 
> The implementation is based on "best effort" approach: if host is
> capable of executing statx(), host statx() is used. If not, the
> implementation includes invoking other (more mature) system calls
> (from the same 'stat' family) on the host side to achieve as close
> as possible functionality.
> 
> Support for statx() in kernel and glibc was, however, introduced
> at different points of time (the difference is more than a year):
> 
>   - kernel: Linux 4.11 (30 April 2017)
>   - glibc: glibc 2.28 (1 Aug 2018)
> 
> In this patch, the availability of statx() support is established
> via __NR_statx (if it is defined, statx() is considered available).
> This coincedes with statx() introduction in kernel.
> 
> However, the structure statx definition may not be available for hosts
> with glibc older than 2.28 (it is, by design, to be defined in one of
> glibc headers), even though the full statx() functionality may be
> supported in kernel, if the kernel is not older than 4.11. Hence,
> a structure "target_statx" is defined in this patch, to remove that
> dependency on glibc headers, and to use statx() functionality as soon
> as the host kernel is capable of supporting it. Such structure statx
> definition is used for both target and host structures statx (of
> course, this doesn't mean the endian arrangement is the same on
> target and host, and endian conversion is done in all necessary
> cases).
> 
> Signed-off-by: Aleksandar Rikalo <arikalo@wavecomp.com>
> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> ---
>  linux-user/syscall.c      | 136 +++++++++++++++++++++++++++++++++++++++++++++-
>  linux-user/syscall_defs.h |  37 +++++++++++++
>  2 files changed, 172 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index d116287..e68a36c 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -43,6 +43,7 @@
>  #include <sys/times.h>
>  #include <sys/shm.h>
>  #include <sys/sem.h>
> +#include <sys/stat.h>
>  #include <sys/statfs.h>
>  #include <utime.h>
>  #include <sys/sysinfo.h>
> @@ -316,6 +317,14 @@ _syscall5(int, kcmp, pid_t, pid1, pid_t, pid2, int, type,
>            unsigned long, idx1, unsigned long, idx2)
>  #endif
>  
> +/*
> + * It is assumed that struct statx is architecture independent.
> + */
> +#if defined(TARGET_NR_statx) && defined(__NR_statx)
> +_syscall5(int, statx, int, dirfd, const char *, pathname, int, flags,
> +          unsigned int, mask, struct target_statx *, statxbuf)
> +#endif
> +
>  static bitmask_transtbl fcntl_flags_tbl[] = {
>    { TARGET_O_ACCMODE,   TARGET_O_WRONLY,    O_ACCMODE,   O_WRONLY,    },
>    { TARGET_O_ACCMODE,   TARGET_O_RDWR,      O_ACCMODE,   O_RDWR,      },
> @@ -6517,6 +6526,48 @@ static inline abi_long host_to_target_stat64(void *cpu_env,
>  }
>  #endif
>  
> +#if defined(TARGET_NR_statx) && defined(__NR_statx)
> +static inline abi_long host_to_target_statx(struct target_statx *host_stx,
> +                                            abi_ulong target_addr)
> +{
> +    struct target_statx *target_stx;
> +
> +    if (!lock_user_struct(VERIFY_WRITE, target_stx, target_addr,  0)) {
> +        return -TARGET_EFAULT;
> +    }
> +    memset(target_stx, 0, sizeof(*target_stx));
> +
> +    __put_user(host_stx->stx_mask, &target_stx->stx_mask);
> +    __put_user(host_stx->stx_blksize, &target_stx->stx_blksize);
> +    __put_user(host_stx->stx_attributes, &target_stx->stx_attributes);
> +    __put_user(host_stx->stx_nlink, &target_stx->stx_nlink);
> +    __put_user(host_stx->stx_uid, &target_stx->stx_uid);
> +    __put_user(host_stx->stx_gid, &target_stx->stx_gid);
> +    __put_user(host_stx->stx_mode, &target_stx->stx_mode);
> +    __put_user(host_stx->stx_ino, &target_stx->stx_ino);
> +    __put_user(host_stx->stx_size, &target_stx->stx_size);
> +    __put_user(host_stx->stx_blocks, &target_stx->stx_blocks);
> +    __put_user(host_stx->stx_attributes_mask, &target_stx->stx_attributes_mask);
> +    __put_user(host_stx->stx_atime.tv_sec, &target_stx->stx_atime.tv_sec);
> +    __put_user(host_stx->stx_atime.tv_nsec, &target_stx->stx_atime.tv_nsec);
> +    __put_user(host_stx->stx_btime.tv_sec, &target_stx->stx_atime.tv_sec);
> +    __put_user(host_stx->stx_btime.tv_nsec, &target_stx->stx_atime.tv_nsec);
> +    __put_user(host_stx->stx_ctime.tv_sec, &target_stx->stx_atime.tv_sec);
> +    __put_user(host_stx->stx_ctime.tv_nsec, &target_stx->stx_atime.tv_nsec);
> +    __put_user(host_stx->stx_mtime.tv_sec, &target_stx->stx_atime.tv_sec);
> +    __put_user(host_stx->stx_mtime.tv_nsec, &target_stx->stx_atime.tv_nsec);
> +    __put_user(host_stx->stx_rdev_major, &target_stx->stx_rdev_major);
> +    __put_user(host_stx->stx_rdev_minor, &target_stx->stx_rdev_minor);
> +    __put_user(host_stx->stx_dev_major, &target_stx->stx_dev_major);
> +    __put_user(host_stx->stx_dev_minor, &target_stx->stx_dev_minor);
> +
> +    unlock_user_struct(target_stx, target_addr, 1);
> +
> +    return 0;
> +}
> +#endif
> +
> +
>  /* ??? Using host futex calls even when target atomic operations
>     are not really atomic probably breaks things.  However implementing
>     futexes locally would make futexes shared between multiple processes
> @@ -7095,7 +7146,8 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>      abi_long ret;
>  #if defined(TARGET_NR_stat) || defined(TARGET_NR_stat64) \
>      || defined(TARGET_NR_lstat) || defined(TARGET_NR_lstat64) \
> -    || defined(TARGET_NR_fstat) || defined(TARGET_NR_fstat64)
> +    || defined(TARGET_NR_fstat) || defined(TARGET_NR_fstat64) \
> +    || defined(TARGET_NR_statx)
>      struct stat st;
>  #endif
>  #if defined(TARGET_NR_statfs) || defined(TARGET_NR_statfs64) \
> @@ -10173,6 +10225,88 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>              ret = host_to_target_stat64(cpu_env, arg3, &st);
>          return ret;
>  #endif
> +#if defined(TARGET_NR_statx)
> +    case TARGET_NR_statx:
> +        {
> +            struct target_statx *target_stx;
> +            int dirfd = arg1;
> +            int flags = arg3;
> +
> +            p = lock_user_string(arg2);
> +            if (p == NULL) {
> +                return -TARGET_EFAULT;
> +            }
> +#if defined(__NR_statx)
> +            {
> +                /*
> +                 * It is assumed that struct statx is architecture independent.
> +                 */
> +                struct target_statx host_stx;
> +                int mask = arg4;
> +
> +                ret = get_errno(statx(dirfd, p, flags, mask, &host_stx));
> +                if (!is_error(ret)) {
> +                    if (host_to_target_statx(&host_stx, arg5) != 0) {
> +                        unlock_user(p, arg2, 0);
> +                        return -TARGET_EFAULT;
> +                    }
> +                }
> +
> +                if (ret != -TARGET_ENOSYS) {
> +                    unlock_user(p, arg2, 0);
> +                    return ret;
> +                }
> +            }
> +#endif
> +            if (*((char *)p) == 0) {
> +                /*
> +                 * By file descriptor
> +                 */
> +                if (flags & AT_EMPTY_PATH) {
> +                    unlock_user(p, arg2, 0);
> +                    return -TARGET_ENOENT;
> +                }
> +                ret = get_errno(fstat(dirfd, &st));
> +            } else if (*((char *)p) == '/') {
> +                /*
> +                 * By absolute pathname
> +                 */
> +                ret = get_errno(stat(path(p), &st));
> +            } else {
> +                /*
> +                 * By pathname relative to the current working directory
> +                 * (if 'dirfd' is AT_FDCWD) or relative to the directory
> +                 * referred to by the file descriptor 'dirfd'.
> +                 */
> +                 ret = get_errno(fstatat(dirfd, path(p), &st, flags));
> +            }
> +            unlock_user(p, arg2, 0);

Could you explain why we can't use fstatat() for the two previous cases
"(*((char *)p) == 0)" and "(*((char *)p) == '/')"?

Thanks,
Laurent



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

* Re: [Qemu-devel] [PATCH v12 5/5] linux-user: Fix flock structure for MIPS O64 ABI
  2019-06-19 14:17 ` [Qemu-devel] [PATCH v12 5/5] linux-user: Fix flock structure for MIPS O64 ABI Aleksandar Markovic
@ 2019-06-19 16:21   ` Laurent Vivier
  2019-06-26  7:54     ` Aleksandar Markovic
  0 siblings, 1 reply; 17+ messages in thread
From: Laurent Vivier @ 2019-06-19 16:21 UTC (permalink / raw)
  To: Aleksandar Markovic, qemu-devel; +Cc: amarkovic

Le 19/06/2019 à 16:17, Aleksandar Markovic a écrit :
> From: Aleksandar Markovic <amarkovic@wavecomp.com>
> 
> Only MIPS O32 and N32 have special (different than other
> architectures) definition of structure flock in kernel.
> 
> Bring flock definition for MIPS O64 ABI to the correct state.
> 
> Reported-by: Dragan Mladjenovic <dmladjenovic@wavecomp.com>
> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> ---
>  linux-user/generic/fcntl.h     | 2 +-
>  linux-user/mips/target_fcntl.h | 4 ++++
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/generic/fcntl.h b/linux-user/generic/fcntl.h
> index a775a49..1b48dde 100644
> --- a/linux-user/generic/fcntl.h
> +++ b/linux-user/generic/fcntl.h
> @@ -129,7 +129,7 @@ struct target_flock {
>      short l_whence;
>      abi_long l_start;
>      abi_long l_len;
> -#if defined(TARGET_MIPS)
> +#if defined(TARGET_MIPS) && (TARGET_ABI_BITS == 32)
>      abi_long l_sysid;
>  #endif
>      int l_pid;
> diff --git a/linux-user/mips/target_fcntl.h b/linux-user/mips/target_fcntl.h
> index 000527c..795bba7 100644
> --- a/linux-user/mips/target_fcntl.h
> +++ b/linux-user/mips/target_fcntl.h
> @@ -27,7 +27,11 @@
>  #define TARGET_F_SETOWN        24       /*  for sockets. */
>  #define TARGET_F_GETOWN        23       /*  for sockets. */
>  
> +#if (TARGET_ABI_BITS == 32)
>  #define TARGET_ARCH_FLOCK_PAD abi_long pad[4];
> +#else
> +#define TARGET_ARCH_FLOCK_PAD
> +#endif
>  #define TARGET_ARCH_FLOCK64_PAD
>  
>  #define TARGET_F_GETLK64       33      /*  using 'struct flock64' */
> 

The patch is correct, but I think it would be cleaner to introduce an
"TARGET_HAVE_ARCH_STRUCT_FLOCK" as we have in the kernel for the mips case.

Thanks,
Laurent


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

* Re: [Qemu-devel] [PATCH v12 2/5] linux-user: Add support for setsockopt() option SOL_ALG
  2019-06-19 14:17 ` [Qemu-devel] [PATCH v12 2/5] linux-user: Add support for setsockopt() option SOL_ALG Aleksandar Markovic
@ 2019-06-24 21:01   ` Laurent Vivier
  0 siblings, 0 replies; 17+ messages in thread
From: Laurent Vivier @ 2019-06-24 21:01 UTC (permalink / raw)
  To: Aleksandar Markovic, qemu-devel; +Cc: Yunqiang Su, amarkovic

Le 19/06/2019 à 16:17, Aleksandar Markovic a écrit :
> From: Yunqiang Su <ysu@wavecomp.com>
> 
> Add support for options SOL_ALG of the syscall setsockopt(). This
> option is used in relation to Linux kernel Crypto API, and allows
> a user to set additional information for the cipher operation via
> syscall setsockopt(). The field "optname" must be one of the
> following:
> 
>   - ALG_SET_KEY – seting the key
>   - ALG_SET_AEAD_AUTHSIZE – set the authentication tag size
> 
> SOL_ALG is relatively newer setsockopt() option. Therefore, the
> code that handles SOL_ALG is enclosed in "ifdef" so that the build
> does not fail for older kernels that do not contain support for
> SOL_ALG. "ifdef" also contains check if ALG_SET_KEY and
> ALG_SET_AEAD_AUTHSIZE are defined.
> 
> Signed-off-by: Yunqiang Su <ysu@wavecomp.com>
> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  linux-user/syscall.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index f267ad0..d116287 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -102,6 +102,7 @@
>  #include <linux/blkpg.h>
>  #include <netpacket/packet.h>
>  #include <linux/netlink.h>
> +#include <linux/if_alg.h>
>  #include "linux_loop.h"
>  #include "uname.h"
>  
> @@ -1989,6 +1990,36 @@ static abi_long do_setsockopt(int sockfd, int level, int optname,
>              goto unimplemented;
>          }
>          break;
> +#if defined(SOL_ALG) && defined(ALG_SET_KEY) && defined(ALG_SET_AEAD_AUTHSIZE)
> +    case SOL_ALG:
> +        switch (optname) {
> +        case ALG_SET_KEY:
> +        {
> +            char *alg_key = g_malloc(optlen);
> +
> +            if (!alg_key) {
> +                return -TARGET_ENOMEM;
> +            }
> +            if (copy_from_user(alg_key, optval_addr, optlen)) {
> +                g_free(alg_key);
> +                return -TARGET_EFAULT;
> +            }
> +            ret = get_errno(setsockopt(sockfd, level, optname,
> +                                       alg_key, optlen));
> +            g_free(alg_key);
> +            break;
> +        }
> +        case ALG_SET_AEAD_AUTHSIZE:
> +        {
> +            ret = get_errno(setsockopt(sockfd, level, optname,
> +                                       NULL, optlen));
> +            break;
> +        }
> +        default:
> +            goto unimplemented;
> +        }
> +        break;
> +#endif
>      case TARGET_SOL_SOCKET:
>          switch (optname) {
>          case TARGET_SO_RCVTIMEO:
> 

Applied to my linux-user branch.

Thanks,
Laurent


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

* Re: [Qemu-devel] [PATCH v12 1/5] linux-user: Add support for setsockopt() options IPV6_<ADD|DROP>_MEMBERSHIP
  2019-06-19 14:17 ` [Qemu-devel] [PATCH v12 1/5] linux-user: Add support for setsockopt() options IPV6_<ADD|DROP>_MEMBERSHIP Aleksandar Markovic
  2019-06-19 16:08   ` Laurent Vivier
@ 2019-06-24 21:04   ` Laurent Vivier
  1 sibling, 0 replies; 17+ messages in thread
From: Laurent Vivier @ 2019-06-24 21:04 UTC (permalink / raw)
  To: Aleksandar Markovic, qemu-devel; +Cc: Neng Chen, amarkovic

Le 19/06/2019 à 16:17, Aleksandar Markovic a écrit :
> From: Neng Chen <nchen@wavecomp.com>
> 
> Add support for the option IPV6_<ADD|DROP>_MEMBERSHIP of the syscall
> setsockopt(). This option controls membership in multicast groups.
> Argument is a pointer to a struct ipv6_mreq.
> 
> The glibc <netinet/in.h> header defines the ipv6_mreq structure,
> which includes the following members:
> 
>   struct in6_addr  ipv6mr_multiaddr;
>   unsigned int     ipv6mr_interface;
> 
> Whereas the kernel in its <linux/in6.h> header defines following
> members of the same structure:
> 
>   struct in6_addr  ipv6mr_multiaddr;
>   int              ipv6mr_ifindex;
> 
> POSIX defines ipv6mr_interface [1].
> 
> __UAPI_DEF_IVP6_MREQ appears in kernel headers with v3.12:
> 
>   cfd280c91253 net: sync some IP headers with glibc
> 
> Without __UAPI_DEF_IVP6_MREQ, kernel defines ipv6mr_ifindex, and
> this is explained in cfd280c91253:
> 
>   "If you include the kernel headers first you get those,
>   and if you include the glibc headers first you get those,
>   and the following patch arranges a coordination and
>   synchronization between the two."
> 
> So before 3.12, a program can't include both <netinet/in.h> and
> <linux/in6.h>.
> 
> In linux-user/syscall.c, we only include <netinet/in.h> (glibc) and
> not <linux/in6.h> (kernel headers), so ipv6mr_interface is the one
> to use.
> 
> [1] http://pubs.opengroup.org/onlinepubs/009695399/basedefs/netinet/in.h.html
> 
> Signed-off-by: Neng Chen <nchen@wavecomp.com>
> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> ---
>  linux-user/syscall.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index b187c12..f267ad0 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1920,6 +1920,25 @@ static abi_long do_setsockopt(int sockfd, int level, int optname,
>                                         &pki, sizeof(pki)));
>              break;
>          }
> +        case IPV6_ADD_MEMBERSHIP:
> +        case IPV6_DROP_MEMBERSHIP:
> +        {
> +            struct ipv6_mreq ipv6mreq;
> +
> +            if (optlen < sizeof(ipv6mreq)) {
> +                return -TARGET_EINVAL;
> +            }
> +
> +            if (copy_from_user(&ipv6mreq, optval_addr, sizeof(ipv6mreq))) {
> +                return -TARGET_EFAULT;
> +            }
> +
> +            ipv6mreq.ipv6mr_interface = tswap32(ipv6mreq.ipv6mr_interface);
> +
> +            ret = get_errno(setsockopt(sockfd, level, optname,
> +                                       &ipv6mreq, sizeof(ipv6mreq)));
> +            break;
> +        }
>          default:
>              goto unimplemented;
>          }
> 

Applied to my linux-user branch.

Thanks,
Laurent


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

* Re: [Qemu-devel] [PATCH v12 5/5] linux-user: Fix flock structure for MIPS O64 ABI
  2019-06-19 16:21   ` Laurent Vivier
@ 2019-06-26  7:54     ` Aleksandar Markovic
  2019-06-26  7:57       ` Laurent Vivier
  0 siblings, 1 reply; 17+ messages in thread
From: Aleksandar Markovic @ 2019-06-26  7:54 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: Aleksandar Markovic, qemu-devel, amarkovic

On Jun 19, 2019 6:34 PM, "Laurent Vivier" <laurent@vivier.eu> wrote:
>
> Le 19/06/2019 à 16:17, Aleksandar Markovic a écrit :
> > From: Aleksandar Markovic <amarkovic@wavecomp.com>
> >
> > Only MIPS O32 and N32 have special (different than other
> > architectures) definition of structure flock in kernel.
> >
> > Bring flock definition for MIPS O64 ABI to the correct state.
> >
> > Reported-by: Dragan Mladjenovic <dmladjenovic@wavecomp.com>
> > Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> > ---
> >  linux-user/generic/fcntl.h     | 2 +-
> >  linux-user/mips/target_fcntl.h | 4 ++++
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/linux-user/generic/fcntl.h b/linux-user/generic/fcntl.h
> > index a775a49..1b48dde 100644
> > --- a/linux-user/generic/fcntl.h
> > +++ b/linux-user/generic/fcntl.h
> > @@ -129,7 +129,7 @@ struct target_flock {
> >      short l_whence;
> >      abi_long l_start;
> >      abi_long l_len;
> > -#if defined(TARGET_MIPS)
> > +#if defined(TARGET_MIPS) && (TARGET_ABI_BITS == 32)
> >      abi_long l_sysid;
> >  #endif
> >      int l_pid;
> > diff --git a/linux-user/mips/target_fcntl.h
b/linux-user/mips/target_fcntl.h
> > index 000527c..795bba7 100644
> > --- a/linux-user/mips/target_fcntl.h
> > +++ b/linux-user/mips/target_fcntl.h
> > @@ -27,7 +27,11 @@
> >  #define TARGET_F_SETOWN        24       /*  for sockets. */
> >  #define TARGET_F_GETOWN        23       /*  for sockets. */
> >
> > +#if (TARGET_ABI_BITS == 32)
> >  #define TARGET_ARCH_FLOCK_PAD abi_long pad[4];
> > +#else
> > +#define TARGET_ARCH_FLOCK_PAD
> > +#endif
> >  #define TARGET_ARCH_FLOCK64_PAD
> >
> >  #define TARGET_F_GETLK64       33      /*  using 'struct flock64' */
> >
>
> The patch is correct, but I think it would be cleaner to introduce an
> "TARGET_HAVE_ARCH_STRUCT_FLOCK" as we have in the kernel for the mips
case.
>
> Thanks,
> Laurent
>

Do you mean we should do everything in a single patch, or do you ask me to
devise a separate restructuring/cleanup patch here?

Aleksandar

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

* Re: [Qemu-devel] [PATCH v12 5/5] linux-user: Fix flock structure for MIPS O64 ABI
  2019-06-26  7:54     ` Aleksandar Markovic
@ 2019-06-26  7:57       ` Laurent Vivier
  0 siblings, 0 replies; 17+ messages in thread
From: Laurent Vivier @ 2019-06-26  7:57 UTC (permalink / raw)
  To: Aleksandar Markovic; +Cc: Aleksandar Markovic, qemu-devel, amarkovic

Le 26/06/2019 à 09:54, Aleksandar Markovic a écrit :
> 
> On Jun 19, 2019 6:34 PM, "Laurent Vivier" <laurent@vivier.eu
> <mailto:laurent@vivier.eu>> wrote:
>>
>> Le 19/06/2019 à 16:17, Aleksandar Markovic a écrit :
>> > From: Aleksandar Markovic <amarkovic@wavecomp.com
> <mailto:amarkovic@wavecomp.com>>
>> >
>> > Only MIPS O32 and N32 have special (different than other
>> > architectures) definition of structure flock in kernel.
>> >
>> > Bring flock definition for MIPS O64 ABI to the correct state.
>> >
>> > Reported-by: Dragan Mladjenovic <dmladjenovic@wavecomp.com
> <mailto:dmladjenovic@wavecomp.com>>
>> > Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com
> <mailto:amarkovic@wavecomp.com>>
>> > ---
>> >  linux-user/generic/fcntl.h     | 2 +-
>> >  linux-user/mips/target_fcntl.h | 4 ++++
>> >  2 files changed, 5 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/linux-user/generic/fcntl.h b/linux-user/generic/fcntl.h
>> > index a775a49..1b48dde 100644
>> > --- a/linux-user/generic/fcntl.h
>> > +++ b/linux-user/generic/fcntl.h
>> > @@ -129,7 +129,7 @@ struct target_flock {
>> >      short l_whence;
>> >      abi_long l_start;
>> >      abi_long l_len;
>> > -#if defined(TARGET_MIPS)
>> > +#if defined(TARGET_MIPS) && (TARGET_ABI_BITS == 32)
>> >      abi_long l_sysid;
>> >  #endif
>> >      int l_pid;
>> > diff --git a/linux-user/mips/target_fcntl.h
> b/linux-user/mips/target_fcntl.h
>> > index 000527c..795bba7 100644
>> > --- a/linux-user/mips/target_fcntl.h
>> > +++ b/linux-user/mips/target_fcntl.h
>> > @@ -27,7 +27,11 @@
>> >  #define TARGET_F_SETOWN        24       /*  for sockets. */
>> >  #define TARGET_F_GETOWN        23       /*  for sockets. */
>> > 
>> > +#if (TARGET_ABI_BITS == 32)
>> >  #define TARGET_ARCH_FLOCK_PAD abi_long pad[4];
>> > +#else
>> > +#define TARGET_ARCH_FLOCK_PAD
>> > +#endif
>> >  #define TARGET_ARCH_FLOCK64_PAD
>> > 
>> >  #define TARGET_F_GETLK64       33      /*  using 'struct flock64' */
>> >
>>
>> The patch is correct, but I think it would be cleaner to introduce an
>> "TARGET_HAVE_ARCH_STRUCT_FLOCK" as we have in the kernel for the mips
> case.
>>
>> Thanks,
>> Laurent
>>
> 
> Do you mean we should do everything in a single patch, or do you ask me
> to devise a separate restructuring/cleanup patch here?

As you want, I'm fine with all in a single patch.

Thanks,
Laurent



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

* Re: [Qemu-devel] [PATCH v12 3/5] linux-user: Add support for translation of statx() syscall
  2019-06-19 16:11   ` Laurent Vivier
@ 2019-06-27 13:18     ` Aleksandar Markovic
  2019-06-27 14:01       ` Laurent Vivier
  0 siblings, 1 reply; 17+ messages in thread
From: Aleksandar Markovic @ 2019-06-27 13:18 UTC (permalink / raw)
  To: Laurent Vivier, Aleksandar Markovic, qemu-devel
  Cc: Aleksandar Rikalo, Jim Wilson

> From: Laurent Vivier <laurent@vivier.eu>
> 
> > @@ -10173,6 +10225,88 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long > arg1,
> >              ret = host_to_target_stat64(cpu_env, arg3, &st);
> >          return ret;
> >  #endif
> > +#if defined(TARGET_NR_statx)
> > +    case TARGET_NR_statx:
> > +        {
> > +            struct target_statx *target_stx;
> > +            int dirfd = arg1;
> > +            int flags = arg3;
> > +
> > +            p = lock_user_string(arg2);
> > +            if (p == NULL) {
> > +                return -TARGET_EFAULT;
> > +            }
> > +#if defined(__NR_statx)
> > +            {
> > +                /*
> > +                 * It is assumed that struct statx is architecture independent.
> > +                 */
> > +                struct target_statx host_stx;
> > +                int mask = arg4;
> > +
> > +                ret = get_errno(statx(dirfd, p, flags, mask, &host_stx));
> > +                if (!is_error(ret)) {
> > +                    if (host_to_target_statx(&host_stx, arg5) != 0) {
> > +                        unlock_user(p, arg2, 0);
> > +                        return -TARGET_EFAULT;
> > +                    }
> > +                }
> > +
> > +                if (ret != -TARGET_ENOSYS) {
> > +                    unlock_user(p, arg2, 0);
> > +                    return ret;
> > +                }
> > +            }
> > +#endif
> > +            if (*((char *)p) == 0) {
> > +                /*
> > +                 * By file descriptor
> > +                 */
> > +                if (flags & AT_EMPTY_PATH) {
> > +                    unlock_user(p, arg2, 0);
> > +                    return -TARGET_ENOENT;
> > +                }
> > +                ret = get_errno(fstat(dirfd, &st));
> > +            } else if (*((char *)p) == '/') {
> > +                /*
> > +                 * By absolute pathname
> > +                 */
> > +                ret = get_errno(stat(path(p), &st));
> > +            } else {
> > +                /*
> > +                 * By pathname relative to the current working directory
> > +                 * (if 'dirfd' is AT_FDCWD) or relative to the directory
> > +                 * referred to by the file descriptor 'dirfd'.
> > +                 */
> > +                 ret = get_errno(fstatat(dirfd, path(p), &st, flags));
> > +            }
> > +            unlock_user(p, arg2, 0);
> 
> Could you explain why we can't use fstatat() for the two previous cases
> "(*((char *)p) == 0)" and "(*((char *)p) == '/')"?
> 

Man page on fstatat (http://man7.org/linux/man-pages/man2/stat.2.html)
says:

   AT_EMPTY_PATH (since Linux 2.6.39)
          If pathname is an empty string, operate on the file referred
          to by dirfd (which may have been obtained using the open(2)
          O_PATH flag).  In this case, dirfd can refer to any type of
          file, not just a directory, and the behavior of fstatat() is
          similar to that of fstat().  If dirfd is AT_FDCWD, the call
          operates on the current working directory.  This flag is
          Linux-specific; define _GNU_SOURCE to obtain its definition.

So it looks the branch "if (*((char *)p) == 0)" can be handled by
fstatat().

Also, the man page says:

   If pathname is absolute, then dirfd is ignored.

So, it looks the case "else if (*((char *)p) == '/')" can also be
handled by fstatat().

Very similar descriptions of the cases above can be found in
the man page for statx
(http://man7.org/linux/man-pages/man2/statx.2.html).

The whole string of if statements after "#endif" above should be now,
in my opinion:

        ret = get_errno(fstatat(dirfd, path(p), &st, flags));
        unlock_user(p, arg2, 0);

... and I will submit the patch with such code, if noone objects.

Yours,
Aleksandar Markovic

> Thanks,
> Laurent



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

* Re: [Qemu-devel] [PATCH v12 3/5] linux-user: Add support for translation of statx() syscall
  2019-06-27 13:18     ` Aleksandar Markovic
@ 2019-06-27 14:01       ` Laurent Vivier
  0 siblings, 0 replies; 17+ messages in thread
From: Laurent Vivier @ 2019-06-27 14:01 UTC (permalink / raw)
  To: Aleksandar Markovic, Aleksandar Markovic, qemu-devel
  Cc: Aleksandar Rikalo, Jim Wilson

Le 27/06/2019 à 15:18, Aleksandar Markovic a écrit :
>> From: Laurent Vivier <laurent@vivier.eu>
>>
>>> @@ -10173,6 +10225,88 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long > arg1,
>>>              ret = host_to_target_stat64(cpu_env, arg3, &st);
>>>          return ret;
>>>  #endif
>>> +#if defined(TARGET_NR_statx)
>>> +    case TARGET_NR_statx:
>>> +        {
>>> +            struct target_statx *target_stx;
>>> +            int dirfd = arg1;
>>> +            int flags = arg3;
>>> +
>>> +            p = lock_user_string(arg2);
>>> +            if (p == NULL) {
>>> +                return -TARGET_EFAULT;
>>> +            }
>>> +#if defined(__NR_statx)
>>> +            {
>>> +                /*
>>> +                 * It is assumed that struct statx is architecture independent.
>>> +                 */
>>> +                struct target_statx host_stx;
>>> +                int mask = arg4;
>>> +
>>> +                ret = get_errno(statx(dirfd, p, flags, mask, &host_stx));
>>> +                if (!is_error(ret)) {
>>> +                    if (host_to_target_statx(&host_stx, arg5) != 0) {
>>> +                        unlock_user(p, arg2, 0);
>>> +                        return -TARGET_EFAULT;
>>> +                    }
>>> +                }
>>> +
>>> +                if (ret != -TARGET_ENOSYS) {
>>> +                    unlock_user(p, arg2, 0);
>>> +                    return ret;
>>> +                }
>>> +            }
>>> +#endif
>>> +            if (*((char *)p) == 0) {
>>> +                /*
>>> +                 * By file descriptor
>>> +                 */
>>> +                if (flags & AT_EMPTY_PATH) {
>>> +                    unlock_user(p, arg2, 0);
>>> +                    return -TARGET_ENOENT;
>>> +                }
>>> +                ret = get_errno(fstat(dirfd, &st));
>>> +            } else if (*((char *)p) == '/') {
>>> +                /*
>>> +                 * By absolute pathname
>>> +                 */
>>> +                ret = get_errno(stat(path(p), &st));
>>> +            } else {
>>> +                /*
>>> +                 * By pathname relative to the current working directory
>>> +                 * (if 'dirfd' is AT_FDCWD) or relative to the directory
>>> +                 * referred to by the file descriptor 'dirfd'.
>>> +                 */
>>> +                 ret = get_errno(fstatat(dirfd, path(p), &st, flags));
>>> +            }
>>> +            unlock_user(p, arg2, 0);
>>
>> Could you explain why we can't use fstatat() for the two previous cases
>> "(*((char *)p) == 0)" and "(*((char *)p) == '/')"?
>>
> 
> Man page on fstatat (http://man7.org/linux/man-pages/man2/stat.2.html)
> says:
> 
>    AT_EMPTY_PATH (since Linux 2.6.39)
>           If pathname is an empty string, operate on the file referred
>           to by dirfd (which may have been obtained using the open(2)
>           O_PATH flag).  In this case, dirfd can refer to any type of
>           file, not just a directory, and the behavior of fstatat() is
>           similar to that of fstat().  If dirfd is AT_FDCWD, the call
>           operates on the current working directory.  This flag is
>           Linux-specific; define _GNU_SOURCE to obtain its definition.
> 
> So it looks the branch "if (*((char *)p) == 0)" can be handled by
> fstatat().
> 
> Also, the man page says:
> 
>    If pathname is absolute, then dirfd is ignored.
> 
> So, it looks the case "else if (*((char *)p) == '/')" can also be
> handled by fstatat().
> 
> Very similar descriptions of the cases above can be found in
> the man page for statx
> (http://man7.org/linux/man-pages/man2/statx.2.html).
> 
> The whole string of if statements after "#endif" above should be now,
> in my opinion:
> 
>         ret = get_errno(fstatat(dirfd, path(p), &st, flags));
>         unlock_user(p, arg2, 0);
> 
> ... and I will submit the patch with such code, if noone objects.
> 

I agree.

Thanks,
Laurent



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

end of thread, other threads:[~2019-06-27 14:06 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-19 14:17 [Qemu-devel] [PATCH v12 0/5] Aleksandar Markovic
2019-06-19 14:17 ` [Qemu-devel] [PATCH v12 1/5] linux-user: Add support for setsockopt() options IPV6_<ADD|DROP>_MEMBERSHIP Aleksandar Markovic
2019-06-19 16:08   ` Laurent Vivier
2019-06-24 21:04   ` Laurent Vivier
2019-06-19 14:17 ` [Qemu-devel] [PATCH v12 2/5] linux-user: Add support for setsockopt() option SOL_ALG Aleksandar Markovic
2019-06-24 21:01   ` Laurent Vivier
2019-06-19 14:17 ` [Qemu-devel] [PATCH v12 3/5] linux-user: Add support for translation of statx() syscall Aleksandar Markovic
2019-06-19 16:11   ` Laurent Vivier
2019-06-27 13:18     ` Aleksandar Markovic
2019-06-27 14:01       ` Laurent Vivier
2019-06-19 14:17 ` [Qemu-devel] [PATCH v12 4/5] linux-user: Add support for strace for " Aleksandar Markovic
2019-06-19 14:17 ` [Qemu-devel] [PATCH v12 5/5] linux-user: Fix flock structure for MIPS O64 ABI Aleksandar Markovic
2019-06-19 16:21   ` Laurent Vivier
2019-06-26  7:54     ` Aleksandar Markovic
2019-06-26  7:57       ` Laurent Vivier
2019-06-19 14:54 ` [Qemu-devel] [PATCH v12 0/5] no-reply
2019-06-19 15:34 ` no-reply

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).