1. Patchew
  2. QEMU
  3. View series

[PATCH v2] linux-user: update select timeout writeback

Sun Haoyu via qemu development posted 1 patch 2 days, 6 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260320055700.44715-1-shyliuli@aosc.io
Maintainers: Laurent Vivier <laurent@vivier.eu>, Pierrick Bouvier <pierrick.bouvier@linaro.org>
There is a newer version of this series
linux-user/syscall.c | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
[PATCH v2] linux-user: update select timeout writeback
Posted by Sun Haoyu via qemu development 2 days, 6 hours ago 
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3343

The Linux kernel writes back the remaining timeout for select-family
syscalls in poll_select_finish(). If that writeback fails, it keeps
the original return value.

However, QEMU only writes back the timeout on success. If the writeback
fails, QEMU returns -TARGET_EFAULT. This can lose the remaining
timeout and change the return value.

Update do_select(), do_pselect6(), and do_ppoll() to always write back
the timeout to match the Linux kernel's behavior. If the timeout
writeback fails, keep the original return value.

Tested with the issue reproducer.

Signed-off-by: Sun Haoyu <shyliuli@aosc.io>
---
Changes since v1:
- write back the timeout in all cases
- handle do_select() too
- keep the original return value if timeout writeback fails

 linux-user/syscall.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 064bc604c9..877a3b8bb6 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1394,13 +1394,12 @@ static abi_long do_select(int n,
             return -TARGET_EFAULT;
         if (efd_addr && copy_to_user_fdset(efd_addr, &efds, n))
             return -TARGET_EFAULT;
-
-        if (target_tv_addr) {
-            tv.tv_sec = ts.tv_sec;
-            tv.tv_usec = ts.tv_nsec / 1000;
-            if (copy_to_user_timeval(target_tv_addr, &tv)) {
-                return -TARGET_EFAULT;
-            }
+    }
+    if (target_tv_addr) {
+        tv.tv_sec = ts.tv_sec;
+        tv.tv_usec = ts.tv_nsec / 1000;
+        if (copy_to_user_timeval(target_tv_addr, &tv)) {
+            return ret;
         }
     }
 
@@ -1529,16 +1528,18 @@ static abi_long do_pselect6(abi_long arg1, abi_long arg2, abi_long arg3,
         if (efd_addr && copy_to_user_fdset(efd_addr, &efds, n)) {
             return -TARGET_EFAULT;
         }
+    }
+    if (ts_addr) {
         if (time64) {
-            if (ts_addr && host_to_target_timespec64(ts_addr, &ts)) {
-                return -TARGET_EFAULT;
+            if (host_to_target_timespec64(ts_addr, &ts)) {
+                return ret;
             }
         } else {
-            if (ts_addr && host_to_target_timespec(ts_addr, &ts)) {
-                return -TARGET_EFAULT;
+            if (host_to_target_timespec(ts_addr, &ts)) {
+                return ret;
             }
         }
-    }
+     }
     return ret;
 }
 #endif
@@ -1606,14 +1607,14 @@ static abi_long do_ppoll(abi_long arg1, abi_long arg2, abi_long arg3,
         if (set) {
             finish_sigsuspend_mask(ret);
         }
-        if (!is_error(ret) && arg3) {
+        if (arg3) {
             if (time64) {
                 if (host_to_target_timespec64(arg3, timeout_ts)) {
-                    return -TARGET_EFAULT;
+                    return ret;
                 }
             } else {
                 if (host_to_target_timespec(arg3, timeout_ts)) {
-                    return -TARGET_EFAULT;
+                    return ret;
                 }
             }
         }
-- 
2.53.0
Re: [PATCH v2] linux-user: update select timeout writeback
Posted by Peter Maydell 2 days, 2 hours ago 
On Fri, 20 Mar 2026 at 05:57, Sun Haoyu <shyliuli@aosc.io> wrote:
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3343

We generally put the Resolves: tag line at the bottom of the
commit message, above the Signed-off-by: line.

> The Linux kernel writes back the remaining timeout for select-family
> syscalls in poll_select_finish(). If that writeback fails, it keeps
> the original return value.
>
> However, QEMU only writes back the timeout on success. If the writeback
> fails, QEMU returns -TARGET_EFAULT. This can lose the remaining
> timeout and change the return value.
>
> Update do_select(), do_pselect6(), and do_ppoll() to always write back
> the timeout to match the Linux kernel's behavior. If the timeout
> writeback fails, keep the original return value.

This surprised me, but it really is what the kernel does:
if the select/poll proper succeeds, it returns the success
value even if the writeback fails:

        /*
         * If an application puts its timeval in read-only memory, we
         * don't want the Linux-specific update to the timeval to
         * cause a fault after the select has completed
         * successfully. However, because we're not updating the
         * timeval, we can't restart the system call.
         */


> Tested with the issue reproducer.
>
> Signed-off-by: Sun Haoyu <shyliuli@aosc.io>
> ---
> Changes since v1:
> - write back the timeout in all cases
> - handle do_select() too
> - keep the original return value if timeout writeback fails
>
>  linux-user/syscall.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 064bc604c9..877a3b8bb6 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1394,13 +1394,12 @@ static abi_long do_select(int n,
>              return -TARGET_EFAULT;
>          if (efd_addr && copy_to_user_fdset(efd_addr, &efds, n))
>              return -TARGET_EFAULT;
> -
> -        if (target_tv_addr) {
> -            tv.tv_sec = ts.tv_sec;
> -            tv.tv_usec = ts.tv_nsec / 1000;
> -            if (copy_to_user_timeval(target_tv_addr, &tv)) {
> -                return -TARGET_EFAULT;
> -            }
> +    }
> +    if (target_tv_addr) {
> +        tv.tv_sec = ts.tv_sec;
> +        tv.tv_usec = ts.tv_nsec / 1000;
> +        if (copy_to_user_timeval(target_tv_addr, &tv)) {
> +            return ret;

We're about to "return ret" anyway, so we can simplify this
by dropping the if(), with a comment about why it's OK:

           /*
            * Like the kernel, we deliberately ignore possible
            * failures writing back to the timeval struct.
            */
           copy_to_user_timeval(target_tv_addr, &tv);

and similarly with the other functions.

>          }
>      }
>
> @@ -1529,16 +1528,18 @@ static abi_long do_pselect6(abi_long arg1, abi_long arg2, abi_long arg3,
>          if (efd_addr && copy_to_user_fdset(efd_addr, &efds, n)) {
>              return -TARGET_EFAULT;
>          }
> +    }
> +    if (ts_addr) {
>          if (time64) {
> -            if (ts_addr && host_to_target_timespec64(ts_addr, &ts)) {
> -                return -TARGET_EFAULT;
> +            if (host_to_target_timespec64(ts_addr, &ts)) {
> +                return ret;
>              }
>          } else {
> -            if (ts_addr && host_to_target_timespec(ts_addr, &ts)) {
> -                return -TARGET_EFAULT;
> +            if (host_to_target_timespec(ts_addr, &ts)) {
> +                return ret;
>              }
>          }
> -    }
> +     }

The indent seems to have been accidentally changed here.

>      return ret;
>  }
>  #endif

Otherwise this looks good to me.

thanks
-- PMM

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.