All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL 0/6] Linux user for 5.2 patches
@ 2020-08-23 14:59 Laurent Vivier
  2020-08-23 14:59 ` [PULL 1/6] linux-user: Fix "print_fdset()" in "strace.c" to not print ", " after last value Laurent Vivier
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Laurent Vivier @ 2020-08-23 14:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier

The following changes since commit d7df0ceee0fd2e512cd214a9074ebeeb40da3099:

  Merge remote-tracking branch 'remotes/philmd-gitlab/tags/sd-next-20200821' =
into staging (2020-08-22 23:53:08 +0100)

are available in the Git repository at:

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

for you to fetch changes up to b3a3af70c377a3e67d43f3be39a333228487b50c:

  linux-user: Fix 'utimensat()' implementation (2020-08-23 16:57:58 +0200)

----------------------------------------------------------------
Add clock_getres_time64, timer_gettime64, timer_settime64,
    timerfd_gettime64, timerfd_settime64
Some fixes (page protection, print_fdset, timerspec, itimerspec)

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

Filip Bozuta (4):
  linux-user: Fix "print_fdset()" in "strace.c" to not print ", " after
    last value
  linux-user: Modify 'target_to_host/host_to_target_itimerspec()'
  linux-user: Add support for a group of 2038 safe syscalls
  linux-user: Fix 'utimensat()' implementation

Richard Henderson (2):
  linux-user: Validate mmap/mprotect prot value
  linux-user: Adjust guest page protection for the host

 linux-user/mmap.c         | 110 +++++++++++++++-------
 linux-user/strace.c       |   8 +-
 linux-user/syscall.c      | 190 ++++++++++++++++++++++++++++++++------
 linux-user/syscall_defs.h |   5 +
 4 files changed, 250 insertions(+), 63 deletions(-)

--=20
2.26.2



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

* [PULL 1/6] linux-user: Fix "print_fdset()" in "strace.c" to not print ", " after last value
  2020-08-23 14:59 [PULL 0/6] Linux user for 5.2 patches Laurent Vivier
@ 2020-08-23 14:59 ` Laurent Vivier
  2020-08-23 14:59 ` [PULL 2/6] linux-user: Validate mmap/mprotect prot value Laurent Vivier
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Laurent Vivier @ 2020-08-23 14:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier, Filip Bozuta

From: Filip Bozuta <Filip.Bozuta@syrmia.com>

Function "print_fdset()" in "strace.c" is used to print the file descriptor
values in "print__newselect()" which prints arguments of syscall _newselect().
Until changes from this patch, this function was printing "," even after the
last value of the fd_set argument. This was changed in this patch by removing
this unnecessary "," after the last fd value and thus improving the estetics of
the _newselect() "-strace" print.

Implementation notes:

   The printing fix was made possible by using an existing function "get_comma()"
   which returns a "," or an empty string "" based on its argument (0 for "," and
   other for "").

Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20200702160915.9517-1-Filip.Bozuta@syrmia.com>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/strace.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 13981341b327..5e380486433c 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -541,6 +541,7 @@ static void
 print_fdset(int n, abi_ulong target_fds_addr)
 {
     int i;
+    int first = 1;
 
     qemu_log("[");
     if( target_fds_addr ) {
@@ -555,9 +556,12 @@ print_fdset(int n, abi_ulong target_fds_addr)
             return;
 
         for (i=n; i>=0; i--) {
-            if ((tswapal(target_fds[i / TARGET_ABI_BITS]) >> (i & (TARGET_ABI_BITS - 1))) & 1)
-                qemu_log("%d,", i);
+            if ((tswapal(target_fds[i / TARGET_ABI_BITS]) >>
+                (i & (TARGET_ABI_BITS - 1))) & 1) {
+                qemu_log("%s%d", get_comma(first), i);
+                first = 0;
             }
+        }
         unlock_user(target_fds, target_fds_addr, 0);
     }
     qemu_log("]");
-- 
2.26.2



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

* [PULL 2/6] linux-user: Validate mmap/mprotect prot value
  2020-08-23 14:59 [PULL 0/6] Linux user for 5.2 patches Laurent Vivier
  2020-08-23 14:59 ` [PULL 1/6] linux-user: Fix "print_fdset()" in "strace.c" to not print ", " after last value Laurent Vivier
@ 2020-08-23 14:59 ` Laurent Vivier
  2020-08-23 14:59 ` [PULL 3/6] linux-user: Adjust guest page protection for the host Laurent Vivier
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Laurent Vivier @ 2020-08-23 14:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Richard Henderson, Laurent Vivier

From: Richard Henderson <richard.henderson@linaro.org>

The kernel will return -EINVAL for bits set in the prot argument
that are unknown or invalid.  Previously we were simply cropping
out the bits that we care about.

Introduce validate_prot_to_pageflags to perform this check in a
single place between the two syscalls.  Differentiate between
the target and host versions of prot.  Compute the qemu internal
page_flags value at the same time.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20200519185645.3915-2-richard.henderson@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/mmap.c | 106 +++++++++++++++++++++++++++++++---------------
 1 file changed, 73 insertions(+), 33 deletions(-)

diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 0019447892e0..46c7eeba9bd2 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -59,64 +59,96 @@ void mmap_fork_end(int child)
         pthread_mutex_unlock(&mmap_mutex);
 }
 
+/*
+ * Validate target prot bitmask.
+ * Return the prot bitmask for the host in *HOST_PROT.
+ * Return 0 if the target prot bitmask is invalid, otherwise
+ * the internal qemu page_flags (which will include PAGE_VALID).
+ */
+static int validate_prot_to_pageflags(int *host_prot, int prot)
+{
+    int valid = PROT_READ | PROT_WRITE | PROT_EXEC | TARGET_PROT_SEM;
+    int page_flags = (prot & PAGE_BITS) | PAGE_VALID;
+
+    /*
+     * For the host, we need not pass anything except read/write/exec.
+     * While PROT_SEM is allowed by all hosts, it is also ignored, so
+     * don't bother transforming guest bit to host bit.  Any other
+     * target-specific prot bits will not be understood by the host
+     * and will need to be encoded into page_flags for qemu emulation.
+     */
+    *host_prot = prot & (PROT_READ | PROT_WRITE | PROT_EXEC);
+
+    return prot & ~valid ? 0 : page_flags;
+}
+
 /* NOTE: all the constants are the HOST ones, but addresses are target. */
-int target_mprotect(abi_ulong start, abi_ulong len, int prot)
+int target_mprotect(abi_ulong start, abi_ulong len, int target_prot)
 {
     abi_ulong end, host_start, host_end, addr;
-    int prot1, ret;
+    int prot1, ret, page_flags, host_prot;
 
-    trace_target_mprotect(start, len, prot);
+    trace_target_mprotect(start, len, target_prot);
 
-    if ((start & ~TARGET_PAGE_MASK) != 0)
+    if ((start & ~TARGET_PAGE_MASK) != 0) {
         return -TARGET_EINVAL;
+    }
+    page_flags = validate_prot_to_pageflags(&host_prot, target_prot);
+    if (!page_flags) {
+        return -TARGET_EINVAL;
+    }
     len = TARGET_PAGE_ALIGN(len);
     end = start + len;
     if (!guest_range_valid(start, len)) {
         return -TARGET_ENOMEM;
     }
-    prot &= PROT_READ | PROT_WRITE | PROT_EXEC;
-    if (len == 0)
+    if (len == 0) {
         return 0;
+    }
 
     mmap_lock();
     host_start = start & qemu_host_page_mask;
     host_end = HOST_PAGE_ALIGN(end);
     if (start > host_start) {
         /* handle host page containing start */
-        prot1 = prot;
-        for(addr = host_start; addr < start; addr += TARGET_PAGE_SIZE) {
+        prot1 = host_prot;
+        for (addr = host_start; addr < start; addr += TARGET_PAGE_SIZE) {
             prot1 |= page_get_flags(addr);
         }
         if (host_end == host_start + qemu_host_page_size) {
-            for(addr = end; addr < host_end; addr += TARGET_PAGE_SIZE) {
+            for (addr = end; addr < host_end; addr += TARGET_PAGE_SIZE) {
                 prot1 |= page_get_flags(addr);
             }
             end = host_end;
         }
-        ret = mprotect(g2h(host_start), qemu_host_page_size, prot1 & PAGE_BITS);
-        if (ret != 0)
+        ret = mprotect(g2h(host_start), qemu_host_page_size,
+                       prot1 & PAGE_BITS);
+        if (ret != 0) {
             goto error;
+        }
         host_start += qemu_host_page_size;
     }
     if (end < host_end) {
-        prot1 = prot;
-        for(addr = end; addr < host_end; addr += TARGET_PAGE_SIZE) {
+        prot1 = host_prot;
+        for (addr = end; addr < host_end; addr += TARGET_PAGE_SIZE) {
             prot1 |= page_get_flags(addr);
         }
-        ret = mprotect(g2h(host_end - qemu_host_page_size), qemu_host_page_size,
-                       prot1 & PAGE_BITS);
-        if (ret != 0)
+        ret = mprotect(g2h(host_end - qemu_host_page_size),
+                       qemu_host_page_size, prot1 & PAGE_BITS);
+        if (ret != 0) {
             goto error;
+        }
         host_end -= qemu_host_page_size;
     }
 
     /* handle the pages in the middle */
     if (host_start < host_end) {
-        ret = mprotect(g2h(host_start), host_end - host_start, prot);
-        if (ret != 0)
+        ret = mprotect(g2h(host_start), host_end - host_start, host_prot);
+        if (ret != 0) {
             goto error;
+        }
     }
-    page_set_flags(start, start + len, prot | PAGE_VALID);
+    page_set_flags(start, start + len, page_flags);
     mmap_unlock();
     return 0;
 error:
@@ -360,19 +392,26 @@ abi_ulong mmap_find_vma(abi_ulong start, abi_ulong size, abi_ulong align)
 }
 
 /* NOTE: all the constants are the HOST ones */
-abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
+abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
                      int flags, int fd, abi_ulong offset)
 {
     abi_ulong ret, end, real_start, real_end, retaddr, host_offset, host_len;
+    int page_flags, host_prot;
 
     mmap_lock();
-    trace_target_mmap(start, len, prot, flags, fd, offset);
+    trace_target_mmap(start, len, target_prot, flags, fd, offset);
 
     if (!len) {
         errno = EINVAL;
         goto fail;
     }
 
+    page_flags = validate_prot_to_pageflags(&host_prot, target_prot);
+    if (!page_flags) {
+        errno = EINVAL;
+        goto fail;
+    }
+
     /* Also check for overflows... */
     len = TARGET_PAGE_ALIGN(len);
     if (!len) {
@@ -438,14 +477,15 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
         /* Note: we prefer to control the mapping address. It is
            especially important if qemu_host_page_size >
            qemu_real_host_page_size */
-        p = mmap(g2h(start), host_len, prot,
+        p = mmap(g2h(start), host_len, host_prot,
                  flags | MAP_FIXED | MAP_ANONYMOUS, -1, 0);
-        if (p == MAP_FAILED)
+        if (p == MAP_FAILED) {
             goto fail;
+        }
         /* update start so that it points to the file position at 'offset' */
         host_start = (unsigned long)p;
         if (!(flags & MAP_ANONYMOUS)) {
-            p = mmap(g2h(start), len, prot,
+            p = mmap(g2h(start), len, host_prot,
                      flags | MAP_FIXED, fd, host_offset);
             if (p == MAP_FAILED) {
                 munmap(g2h(start), host_len);
@@ -479,19 +519,19 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
             /* msync() won't work here, so we return an error if write is
                possible while it is a shared mapping */
             if ((flags & MAP_TYPE) == MAP_SHARED &&
-                (prot & PROT_WRITE)) {
+                (host_prot & PROT_WRITE)) {
                 errno = EINVAL;
                 goto fail;
             }
-            retaddr = target_mmap(start, len, prot | PROT_WRITE,
+            retaddr = target_mmap(start, len, target_prot | PROT_WRITE,
                                   MAP_FIXED | MAP_PRIVATE | MAP_ANONYMOUS,
                                   -1, 0);
             if (retaddr == -1)
                 goto fail;
             if (pread(fd, g2h(start), len, offset) == -1)
                 goto fail;
-            if (!(prot & PROT_WRITE)) {
-                ret = target_mprotect(start, len, prot);
+            if (!(host_prot & PROT_WRITE)) {
+                ret = target_mprotect(start, len, target_prot);
                 assert(ret == 0);
             }
             goto the_end;
@@ -502,13 +542,13 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
             if (real_end == real_start + qemu_host_page_size) {
                 /* one single host page */
                 ret = mmap_frag(real_start, start, end,
-                                prot, flags, fd, offset);
+                                host_prot, flags, fd, offset);
                 if (ret == -1)
                     goto fail;
                 goto the_end1;
             }
             ret = mmap_frag(real_start, start, real_start + qemu_host_page_size,
-                            prot, flags, fd, offset);
+                            host_prot, flags, fd, offset);
             if (ret == -1)
                 goto fail;
             real_start += qemu_host_page_size;
@@ -517,7 +557,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
         if (end < real_end) {
             ret = mmap_frag(real_end - qemu_host_page_size,
                             real_end - qemu_host_page_size, end,
-                            prot, flags, fd,
+                            host_prot, flags, fd,
                             offset + real_end - qemu_host_page_size - start);
             if (ret == -1)
                 goto fail;
@@ -533,13 +573,13 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
             else
                 offset1 = offset + real_start - start;
             p = mmap(g2h(real_start), real_end - real_start,
-                     prot, flags, fd, offset1);
+                     host_prot, flags, fd, offset1);
             if (p == MAP_FAILED)
                 goto fail;
         }
     }
  the_end1:
-    page_set_flags(start, start + len, prot | PAGE_VALID);
+    page_set_flags(start, start + len, page_flags);
  the_end:
     trace_target_mmap_complete(start);
     if (qemu_loglevel_mask(CPU_LOG_PAGE)) {
-- 
2.26.2



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

* [PULL 3/6] linux-user: Adjust guest page protection for the host
  2020-08-23 14:59 [PULL 0/6] Linux user for 5.2 patches Laurent Vivier
  2020-08-23 14:59 ` [PULL 1/6] linux-user: Fix "print_fdset()" in "strace.c" to not print ", " after last value Laurent Vivier
  2020-08-23 14:59 ` [PULL 2/6] linux-user: Validate mmap/mprotect prot value Laurent Vivier
@ 2020-08-23 14:59 ` Laurent Vivier
  2020-08-23 14:59 ` [PULL 4/6] linux-user: Modify 'target_to_host/host_to_target_itimerspec()' Laurent Vivier
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Laurent Vivier @ 2020-08-23 14:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Richard Henderson, Laurent Vivier,
	Philippe Mathieu-Daudé

From: Richard Henderson <richard.henderson@linaro.org>

Executable guest pages are never directly executed by
the host, but do need to be readable for translation.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20200519185645.3915-3-richard.henderson@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/mmap.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 46c7eeba9bd2..f2615634201d 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -76,8 +76,12 @@ static int validate_prot_to_pageflags(int *host_prot, int prot)
      * don't bother transforming guest bit to host bit.  Any other
      * target-specific prot bits will not be understood by the host
      * and will need to be encoded into page_flags for qemu emulation.
+     *
+     * Pages that are executable by the guest will never be executed
+     * by the host, but the host will need to be able to read them.
      */
-    *host_prot = prot & (PROT_READ | PROT_WRITE | PROT_EXEC);
+    *host_prot = (prot & (PROT_READ | PROT_WRITE))
+               | (prot & PROT_EXEC ? PROT_READ : 0);
 
     return prot & ~valid ? 0 : page_flags;
 }
-- 
2.26.2



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

* [PULL 4/6] linux-user: Modify 'target_to_host/host_to_target_itimerspec()'
  2020-08-23 14:59 [PULL 0/6] Linux user for 5.2 patches Laurent Vivier
                   ` (2 preceding siblings ...)
  2020-08-23 14:59 ` [PULL 3/6] linux-user: Adjust guest page protection for the host Laurent Vivier
@ 2020-08-23 14:59 ` Laurent Vivier
  2020-08-23 14:59 ` [PULL 5/6] linux-user: Add support for a group of 2038 safe syscalls Laurent Vivier
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Laurent Vivier @ 2020-08-23 14:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier, Filip Bozuta

From: Filip Bozuta <Filip.Bozuta@syrmia.com>

Functions 'target_to_host_itimerspec()' and 'host_to_target_itimerspec()'
are used to convert values of type 'struct itimerspec' between target and
host. This type has 'struct timespec' as its fields. That is the reason
why this patch introduces a little modification to the converting functions
to be implemented using already existing functions that convert 'struct timespec':
'target_to_host_timespec()' and 'host_to_target_timespec()'. This makes the
code of 'target_to_host_itimerspec()' and 'host_to_target_itimerspec()' more
clean and readable.

Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20200722153421.295411-2-Filip.Bozuta@syrmia.com>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/syscall.c | 46 ++++++++++++++++++--------------------------
 1 file changed, 19 insertions(+), 27 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 945fc252791c..aea1160804a2 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1229,7 +1229,9 @@ static inline abi_long copy_to_user_timeval64(abi_ulong target_tv_addr,
     defined(TARGET_NR_nanosleep) || defined(TARGET_NR_clock_settime) || \
     defined(TARGET_NR_utimensat) || defined(TARGET_NR_mq_timedsend) || \
     defined(TARGET_NR_mq_timedreceive) || defined(TARGET_NR_ipc) || \
-    defined(TARGET_NR_semop) || defined(TARGET_NR_semtimedop)
+    defined(TARGET_NR_semop) || defined(TARGET_NR_semtimedop) || \
+    defined(TARGET_NR_timer_settime) || \
+    (defined(TARGET_NR_timerfd_settime) && defined(CONFIG_TIMERFD))
 static inline abi_long target_to_host_timespec(struct timespec *host_ts,
                                                abi_ulong target_addr)
 {
@@ -6783,46 +6785,36 @@ static inline abi_long target_ftruncate64(void *cpu_env, abi_long arg1,
 
 #if defined(TARGET_NR_timer_settime) || \
     (defined(TARGET_NR_timerfd_settime) && defined(CONFIG_TIMERFD))
-static inline abi_long target_to_host_itimerspec(struct itimerspec *host_itspec,
+static inline abi_long target_to_host_itimerspec(struct itimerspec *host_its,
                                                  abi_ulong target_addr)
 {
-    struct target_itimerspec *target_itspec;
-
-    if (!lock_user_struct(VERIFY_READ, target_itspec, target_addr, 1)) {
+    if (target_to_host_timespec(&host_its->it_interval, target_addr +
+                                offsetof(struct target_itimerspec,
+                                         it_interval)) ||
+        target_to_host_timespec(&host_its->it_value, target_addr +
+                                offsetof(struct target_itimerspec,
+                                         it_value))) {
         return -TARGET_EFAULT;
     }
 
-    host_itspec->it_interval.tv_sec =
-                            tswapal(target_itspec->it_interval.tv_sec);
-    host_itspec->it_interval.tv_nsec =
-                            tswapal(target_itspec->it_interval.tv_nsec);
-    host_itspec->it_value.tv_sec = tswapal(target_itspec->it_value.tv_sec);
-    host_itspec->it_value.tv_nsec = tswapal(target_itspec->it_value.tv_nsec);
-
-    unlock_user_struct(target_itspec, target_addr, 1);
     return 0;
 }
 #endif
 
 #if ((defined(TARGET_NR_timerfd_gettime) || \
       defined(TARGET_NR_timerfd_settime)) && defined(CONFIG_TIMERFD)) || \
-    defined(TARGET_NR_timer_gettime) || defined(TARGET_NR_timer_settime)
+      defined(TARGET_NR_timer_gettime) || defined(TARGET_NR_timer_settime)
 static inline abi_long host_to_target_itimerspec(abi_ulong target_addr,
-                                               struct itimerspec *host_its)
-{
-    struct target_itimerspec *target_itspec;
-
-    if (!lock_user_struct(VERIFY_WRITE, target_itspec, target_addr, 0)) {
+                                                 struct itimerspec *host_its)
+{
+    if (host_to_target_timespec(target_addr + offsetof(struct target_itimerspec,
+                                                       it_interval),
+                                &host_its->it_interval) ||
+        host_to_target_timespec(target_addr + offsetof(struct target_itimerspec,
+                                                       it_value),
+                                &host_its->it_value)) {
         return -TARGET_EFAULT;
     }
-
-    target_itspec->it_interval.tv_sec = tswapal(host_its->it_interval.tv_sec);
-    target_itspec->it_interval.tv_nsec = tswapal(host_its->it_interval.tv_nsec);
-
-    target_itspec->it_value.tv_sec = tswapal(host_its->it_value.tv_sec);
-    target_itspec->it_value.tv_nsec = tswapal(host_its->it_value.tv_nsec);
-
-    unlock_user_struct(target_itspec, target_addr, 0);
     return 0;
 }
 #endif
-- 
2.26.2



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

* [PULL 5/6] linux-user: Add support for a group of 2038 safe syscalls
  2020-08-23 14:59 [PULL 0/6] Linux user for 5.2 patches Laurent Vivier
                   ` (3 preceding siblings ...)
  2020-08-23 14:59 ` [PULL 4/6] linux-user: Modify 'target_to_host/host_to_target_itimerspec()' Laurent Vivier
@ 2020-08-23 14:59 ` Laurent Vivier
  2020-08-23 14:59 ` [PULL 6/6] linux-user: Fix 'utimensat()' implementation Laurent Vivier
  2020-08-23 16:52 ` [PULL 0/6] Linux user for 5.2 patches Peter Maydell
  6 siblings, 0 replies; 8+ messages in thread
From: Laurent Vivier @ 2020-08-23 14:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier, Filip Bozuta

From: Filip Bozuta <Filip.Bozuta@syrmia.com>

This patch implements functionality for following time64 syscalls:

*clock_getres_time64

     This a year 2038 safe variant of syscall:

     int clock_getres(clockid_t clockid, struct timespec *res)
     --finding the resoultion of a specified clock--
     man page: https://man7.org/linux/man-pages/man2/clock_getres.2.html

*timer_gettime64
*timer_settime64

     These are year 2038 safe variants of syscalls:

     int timer_settime(timer_t timerid, int flags,
                       const struct itimerspec *new_value,
                       struct itimerspec *old_value)
     int timer_gettime(timer_t timerid, struct itimerspec *curr_value)
     --arming/dissarming and fetching state of POSIX per-process timer--
     man page: https://man7.org/linux/man-pages/man2/timer_settime.2.html

*timerfd_gettime64
*timerfd_settime64

     These are year 2038 safe variants of syscalls:

     int timerfd_settime(int fd, int flags,
                         const struct itimerspec *new_value,
                         struct itimerspec *old_value)
     int timerfd_gettime(int fd, struct itimerspec *curr_value)
     --timers that notify via file descriptor--
     man page: https://man7.org/linux/man-pages/man2/timerfd_settime.2.html

Implementation notes:

     Syscall 'clock_getres_time64' was implemented similarly to 'clock_getres()'.
     The only difference was that for the conversion of 'struct timespec' from
     host to target, function 'host_to_target_timespec64()' was used instead of
     'host_to_target_timespec()'.

     For other syscalls, new functions 'host_to_target_itimerspec64()' and
     'target_to_host_itimerspec64()' were added to convert the value of the
     'struct itimerspec' from host to target and vice versa. A new type
     'struct target__kernel_itimerspec' was added in 'syscall_defs.h'. This
     type was defined with fields which are of the already defined type
     'struct target_timespec'. This new 'struct target__kernel_itimerspec'
     type is used in these new converting functions. These new functions were
     defined similarly to 'host_to_target_itimerspec()' and 'target_to_host_itimerspec()'
     the only difference being that 'target_to_host_timespec64()' and
     'host_to_target_timespec64()' were used.

Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20200722153421.295411-3-Filip.Bozuta@syrmia.com>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/syscall.c      | 139 +++++++++++++++++++++++++++++++++++++-
 linux-user/syscall_defs.h |   5 ++
 2 files changed, 143 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index aea1160804a2..bbb61a59c72f 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1247,7 +1247,9 @@ static inline abi_long target_to_host_timespec(struct timespec *host_ts,
 }
 #endif
 
-#if defined(TARGET_NR_clock_settime64) || defined(TARGET_NR_futex_time64)
+#if defined(TARGET_NR_clock_settime64) || defined(TARGET_NR_futex_time64) || \
+    defined(TARGET_NR_timer_settime64) || \
+    (defined(TARGET_NR_timerfd_settime64) && defined(CONFIG_TIMERFD))
 static inline abi_long target_to_host_timespec64(struct timespec *host_ts,
                                                  abi_ulong target_addr)
 {
@@ -6801,6 +6803,24 @@ static inline abi_long target_to_host_itimerspec(struct itimerspec *host_its,
 }
 #endif
 
+#if defined(TARGET_NR_timer_settime64) || \
+    (defined(TARGET_NR_timerfd_settime64) && defined(CONFIG_TIMERFD))
+static inline abi_long target_to_host_itimerspec64(struct itimerspec *host_its,
+                                                   abi_ulong target_addr)
+{
+    if (target_to_host_timespec64(&host_its->it_interval, target_addr +
+                                  offsetof(struct target__kernel_itimerspec,
+                                           it_interval)) ||
+        target_to_host_timespec64(&host_its->it_value, target_addr +
+                                  offsetof(struct target__kernel_itimerspec,
+                                           it_value))) {
+        return -TARGET_EFAULT;
+    }
+
+    return 0;
+}
+#endif
+
 #if ((defined(TARGET_NR_timerfd_gettime) || \
       defined(TARGET_NR_timerfd_settime)) && defined(CONFIG_TIMERFD)) || \
       defined(TARGET_NR_timer_gettime) || defined(TARGET_NR_timer_settime)
@@ -6819,6 +6839,26 @@ static inline abi_long host_to_target_itimerspec(abi_ulong target_addr,
 }
 #endif
 
+#if ((defined(TARGET_NR_timerfd_gettime64) || \
+      defined(TARGET_NR_timerfd_settime64)) && defined(CONFIG_TIMERFD)) || \
+      defined(TARGET_NR_timer_gettime64) || defined(TARGET_NR_timer_settime64)
+static inline abi_long host_to_target_itimerspec64(abi_ulong target_addr,
+                                                   struct itimerspec *host_its)
+{
+    if (host_to_target_timespec64(target_addr +
+                                  offsetof(struct target__kernel_itimerspec,
+                                           it_interval),
+                                  &host_its->it_interval) ||
+        host_to_target_timespec64(target_addr +
+                                  offsetof(struct target__kernel_itimerspec,
+                                           it_value),
+                                  &host_its->it_value)) {
+        return -TARGET_EFAULT;
+    }
+    return 0;
+}
+#endif
+
 #if defined(TARGET_NR_adjtimex) || \
     (defined(TARGET_NR_clock_adjtime) && defined(CONFIG_CLOCK_ADJTIME))
 static inline abi_long target_to_host_timex(struct timex *host_tx,
@@ -11811,6 +11851,17 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
         return ret;
     }
 #endif
+#ifdef TARGET_NR_clock_getres_time64
+    case TARGET_NR_clock_getres_time64:
+    {
+        struct timespec ts;
+        ret = get_errno(clock_getres(arg1, &ts));
+        if (!is_error(ret)) {
+            host_to_target_timespec64(arg2, &ts);
+        }
+        return ret;
+    }
+#endif
 #ifdef TARGET_NR_clock_nanosleep
     case TARGET_NR_clock_nanosleep:
     {
@@ -12405,6 +12456,32 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
     }
 #endif
 
+#ifdef TARGET_NR_timer_settime64
+    case TARGET_NR_timer_settime64:
+    {
+        target_timer_t timerid = get_timer_id(arg1);
+
+        if (timerid < 0) {
+            ret = timerid;
+        } else if (arg3 == 0) {
+            ret = -TARGET_EINVAL;
+        } else {
+            timer_t htimer = g_posix_timers[timerid];
+            struct itimerspec hspec_new = {{0},}, hspec_old = {{0},};
+
+            if (target_to_host_itimerspec64(&hspec_new, arg3)) {
+                return -TARGET_EFAULT;
+            }
+            ret = get_errno(
+                          timer_settime(htimer, arg2, &hspec_new, &hspec_old));
+            if (arg4 && host_to_target_itimerspec64(arg4, &hspec_old)) {
+                return -TARGET_EFAULT;
+            }
+        }
+        return ret;
+    }
+#endif
+
 #ifdef TARGET_NR_timer_gettime
     case TARGET_NR_timer_gettime:
     {
@@ -12428,6 +12505,29 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
     }
 #endif
 
+#ifdef TARGET_NR_timer_gettime64
+    case TARGET_NR_timer_gettime64:
+    {
+        /* args: timer_t timerid, struct itimerspec64 *curr_value */
+        target_timer_t timerid = get_timer_id(arg1);
+
+        if (timerid < 0) {
+            ret = timerid;
+        } else if (!arg2) {
+            ret = -TARGET_EFAULT;
+        } else {
+            timer_t htimer = g_posix_timers[timerid];
+            struct itimerspec hspec;
+            ret = get_errno(timer_gettime(htimer, &hspec));
+
+            if (host_to_target_itimerspec64(arg2, &hspec)) {
+                ret = -TARGET_EFAULT;
+            }
+        }
+        return ret;
+    }
+#endif
+
 #ifdef TARGET_NR_timer_getoverrun
     case TARGET_NR_timer_getoverrun:
     {
@@ -12481,6 +12581,20 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
         return ret;
 #endif
 
+#if defined(TARGET_NR_timerfd_gettime64) && defined(CONFIG_TIMERFD)
+    case TARGET_NR_timerfd_gettime64:
+        {
+            struct itimerspec its_curr;
+
+            ret = get_errno(timerfd_gettime(arg1, &its_curr));
+
+            if (arg2 && host_to_target_itimerspec64(arg2, &its_curr)) {
+                return -TARGET_EFAULT;
+            }
+        }
+        return ret;
+#endif
+
 #if defined(TARGET_NR_timerfd_settime) && defined(CONFIG_TIMERFD)
     case TARGET_NR_timerfd_settime:
         {
@@ -12504,6 +12618,29 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
         return ret;
 #endif
 
+#if defined(TARGET_NR_timerfd_settime64) && defined(CONFIG_TIMERFD)
+    case TARGET_NR_timerfd_settime64:
+        {
+            struct itimerspec its_new, its_old, *p_new;
+
+            if (arg3) {
+                if (target_to_host_itimerspec64(&its_new, arg3)) {
+                    return -TARGET_EFAULT;
+                }
+                p_new = &its_new;
+            } else {
+                p_new = NULL;
+            }
+
+            ret = get_errno(timerfd_settime(arg1, arg2, p_new, &its_old));
+
+            if (arg4 && host_to_target_itimerspec64(arg4, &its_old)) {
+                return -TARGET_EFAULT;
+            }
+        }
+        return ret;
+#endif
+
 #if defined(TARGET_NR_ioprio_get) && defined(__NR_ioprio_get)
     case TARGET_NR_ioprio_get:
         return get_errno(ioprio_get(arg1, arg2));
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 3c261cff0e5e..427a25f5bce5 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -259,6 +259,11 @@ struct target_itimerspec {
     struct target_timespec it_value;
 };
 
+struct target__kernel_itimerspec {
+    struct target__kernel_timespec it_interval;
+    struct target__kernel_timespec it_value;
+};
+
 struct target_timex {
     abi_uint modes;              /* Mode selector */
     abi_long offset;             /* Time offset */
-- 
2.26.2



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

* [PULL 6/6] linux-user: Fix 'utimensat()' implementation
  2020-08-23 14:59 [PULL 0/6] Linux user for 5.2 patches Laurent Vivier
                   ` (4 preceding siblings ...)
  2020-08-23 14:59 ` [PULL 5/6] linux-user: Add support for a group of 2038 safe syscalls Laurent Vivier
@ 2020-08-23 14:59 ` Laurent Vivier
  2020-08-23 16:52 ` [PULL 0/6] Linux user for 5.2 patches Peter Maydell
  6 siblings, 0 replies; 8+ messages in thread
From: Laurent Vivier @ 2020-08-23 14:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier, Filip Bozuta

From: Filip Bozuta <Filip.Bozuta@syrmia.com>

Implementation of syscall 'utimensat()' in 'syscall.c' uses functions
target_to_host/host_to_target_timespec() to convert values of
'struct timespec' between host and target. However, the implementation
doesn't check whether the conversion succeeds and thus can cause an
inappropriate error or succeed unappropriately instead of setting errno
EFAULT ('Bad address') which is supposed to be set in these cases.

This was confirmed with the LTP test for utimensat ('testcases/utimensat')
which fails for test cases when the errno EFAULT is expected. After changes
from this patch, the test passes for all test cases.

Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20200811113101.6636-1-Filip.Bozuta@syrmia.com>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/syscall.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index bbb61a59c72f..b4a7b605f3d4 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -11919,8 +11919,13 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
             if (!arg3) {
                 tsp = NULL;
             } else {
-                target_to_host_timespec(ts, arg3);
-                target_to_host_timespec(ts+1, arg3+sizeof(struct target_timespec));
+                if (target_to_host_timespec(ts, arg3)) {
+                    return -TARGET_EFAULT;
+                }
+                if (target_to_host_timespec(ts + 1, arg3 +
+                                            sizeof(struct target_timespec))) {
+                    return -TARGET_EFAULT;
+                }
                 tsp = ts;
             }
             if (!arg2)
-- 
2.26.2



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

* Re: [PULL 0/6] Linux user for 5.2 patches
  2020-08-23 14:59 [PULL 0/6] Linux user for 5.2 patches Laurent Vivier
                   ` (5 preceding siblings ...)
  2020-08-23 14:59 ` [PULL 6/6] linux-user: Fix 'utimensat()' implementation Laurent Vivier
@ 2020-08-23 16:52 ` Peter Maydell
  6 siblings, 0 replies; 8+ messages in thread
From: Peter Maydell @ 2020-08-23 16:52 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: QEMU Developers

On Sun, 23 Aug 2020 at 16:00, Laurent Vivier <laurent@vivier.eu> wrote:
>
> The following changes since commit d7df0ceee0fd2e512cd214a9074ebeeb40da3099:
>
>   Merge remote-tracking branch 'remotes/philmd-gitlab/tags/sd-next-20200821' =
> into staging (2020-08-22 23:53:08 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/vivier/qemu.git tags/linux-user-for-5.2-pull-request
>
> for you to fetch changes up to b3a3af70c377a3e67d43f3be39a333228487b50c:
>
>   linux-user: Fix 'utimensat()' implementation (2020-08-23 16:57:58 +0200)
>
> ----------------------------------------------------------------
> Add clock_getres_time64, timer_gettime64, timer_settime64,
>     timerfd_gettime64, timerfd_settime64
> Some fixes (page protection, print_fdset, timerspec, itimerspec)
>
> ----------------------------------------------------------------


Applied, thanks.

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

-- PMM


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

end of thread, other threads:[~2020-08-23 16:53 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-23 14:59 [PULL 0/6] Linux user for 5.2 patches Laurent Vivier
2020-08-23 14:59 ` [PULL 1/6] linux-user: Fix "print_fdset()" in "strace.c" to not print ", " after last value Laurent Vivier
2020-08-23 14:59 ` [PULL 2/6] linux-user: Validate mmap/mprotect prot value Laurent Vivier
2020-08-23 14:59 ` [PULL 3/6] linux-user: Adjust guest page protection for the host Laurent Vivier
2020-08-23 14:59 ` [PULL 4/6] linux-user: Modify 'target_to_host/host_to_target_itimerspec()' Laurent Vivier
2020-08-23 14:59 ` [PULL 5/6] linux-user: Add support for a group of 2038 safe syscalls Laurent Vivier
2020-08-23 14:59 ` [PULL 6/6] linux-user: Fix 'utimensat()' implementation Laurent Vivier
2020-08-23 16:52 ` [PULL 0/6] Linux user for 5.2 patches Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.