[PATCH 0/5] linux-user: signal mask fixes for pselect et al

[PATCH 0/5] linux-user: signal mask fixes for pselect et al

[Richard Henderson]

Split out some helpers from sigsuspend.
Reuse them for pselect, epoll_pwait, ppoll.
Fix an alpha buglet found on the way.

r~

Richard Henderson (5):
  linux-user/alpha: Fix sigsuspend for big-endian hosts
  linux-user: Split out helpers for sigsuspend
  linux-user: Properly handle sigset arg to pselect
  linux-user: Properly handle sigset arg to epoll_pwait
  linux-user: Properly handle sigset arg to ppoll

 linux-user/signal-common.h |  26 ++++++++
 linux-user/signal.c        |  23 +++++++
 linux-user/syscall.c       | 119 ++++++++++++++-----------------------
 3 files changed, 92 insertions(+), 76 deletions(-)

-- 
2.25.1

[PATCH 1/5] linux-user/alpha: Fix sigsuspend for big-endian hosts

[Richard Henderson]

On alpha, the sigset argument for sigsuspend is in a register.
When we drop that into memory that happens in host-endianness,
but target_to_host_old_sigset will treat it as target-endianness.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/syscall.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index b9b18a7eaf..ecd00382a8 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9559,7 +9559,8 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
         {
             TaskState *ts = cpu->opaque;
 #if defined(TARGET_ALPHA)
-            abi_ulong mask = arg1;
+            /* target_to_host_old_sigset will bswap back */
+            abi_ulong mask = tswapal(arg1);
             target_to_host_old_sigset(&ts->sigsuspend_mask, &mask);
 #else
             if (!(p = lock_user(VERIFY_READ, arg1, sizeof(target_sigset_t), 1)))
-- 
2.25.1

Re: [PATCH 1/5] linux-user/alpha: Fix sigsuspend for big-endian hosts

[Laurent Vivier]

Le 15/03/2022 à 09:43, Richard Henderson a écrit :
> On alpha, the sigset argument for sigsuspend is in a register.
> When we drop that into memory that happens in host-endianness,
> but target_to_host_old_sigset will treat it as target-endianness.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/syscall.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index b9b18a7eaf..ecd00382a8 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -9559,7 +9559,8 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>           {
>               TaskState *ts = cpu->opaque;
>   #if defined(TARGET_ALPHA)
> -            abi_ulong mask = arg1;
> +            /* target_to_host_old_sigset will bswap back */
> +            abi_ulong mask = tswapal(arg1);
>               target_to_host_old_sigset(&ts->sigsuspend_mask, &mask);
>   #else
>               if (!(p = lock_user(VERIFY_READ, arg1, sizeof(target_sigset_t), 1)))

And what about target_set in TARGET_NR_ssetmask, mask in TARGET_NR_sigprocmask and in 
TARGET_NR_osf_sigprocmask?

Thanks,
Laurent

Re: [PATCH 1/5] linux-user/alpha: Fix sigsuspend for big-endian hosts

[Laurent Vivier]

Le 22/03/2022 à 10:58, Laurent Vivier a écrit :
> Le 15/03/2022 à 09:43, Richard Henderson a écrit :
>> On alpha, the sigset argument for sigsuspend is in a register.
>> When we drop that into memory that happens in host-endianness,
>> but target_to_host_old_sigset will treat it as target-endianness.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   linux-user/syscall.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index b9b18a7eaf..ecd00382a8 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -9559,7 +9559,8 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>>           {
>>               TaskState *ts = cpu->opaque;
>>   #if defined(TARGET_ALPHA)
>> -            abi_ulong mask = arg1;
>> +            /* target_to_host_old_sigset will bswap back */
>> +            abi_ulong mask = tswapal(arg1);
>>               target_to_host_old_sigset(&ts->sigsuspend_mask, &mask);
>>   #else
>>               if (!(p = lock_user(VERIFY_READ, arg1, sizeof(target_sigset_t), 1)))
> 
> And what about target_set in TARGET_NR_ssetmask, mask in TARGET_NR_sigprocmask and in 
> TARGET_NR_osf_sigprocmask?
> 

Anyway, the fix is correct and I will add it in my next pull request.

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

[PATCH 2/5] linux-user: Split out helpers for sigsuspend

[Richard Henderson]

Two new functions: process_sigsuspend_mask and finish_sigsuspend_mask.
Move the size check and copy-from-user code.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/signal-common.h | 26 +++++++++++++++++++++++++
 linux-user/signal.c        | 23 ++++++++++++++++++++++
 linux-user/syscall.c       | 40 ++++++++++++++++----------------------
 3 files changed, 66 insertions(+), 23 deletions(-)

diff --git a/linux-user/signal-common.h b/linux-user/signal-common.h
index 2113165a75..6a7e4a93fc 100644
--- a/linux-user/signal-common.h
+++ b/linux-user/signal-common.h
@@ -92,4 +92,30 @@ abi_long do_swapcontext(CPUArchState *env, abi_ulong uold_ctx,
  */
 int block_signals(void); /* Returns non zero if signal pending */
 
+/**
+ * process_sigsuspend_mask: read and apply syscall-local signal mask
+ *
+ * Read the guest signal mask from @sigset, length @sigsize.
+ * Convert that to a host signal mask and save it to sigpending_mask.
+ *
+ * Return value: negative target errno, or zero;
+ *               store &sigpending_mask into *pset on success.
+ */
+int process_sigsuspend_mask(sigset_t **pset, target_ulong sigset,
+                            target_ulong sigsize);
+
+/**
+ * finish_sigsuspend_mask: finish a sigsuspend-like syscall
+ *
+ * Set in_sigsuspend if we need to use the modified sigset
+ * during process_pending_signals.
+ */
+static inline void finish_sigsuspend_mask(int ret)
+{
+    if (ret != -QEMU_ERESTARTSYS) {
+        TaskState *ts = (TaskState *)thread_cpu->opaque;
+        ts->in_sigsuspend = 1;
+    }
+}
+
 #endif
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 2a3f3cc23f..092e70b80c 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -1199,3 +1199,26 @@ void process_pending_signals(CPUArchState *cpu_env)
     }
     ts->in_sigsuspend = 0;
 }
+
+int process_sigsuspend_mask(sigset_t **pset, target_ulong sigset,
+                            target_ulong sigsize)
+{
+    TaskState *ts = (TaskState *)thread_cpu->opaque;
+    sigset_t *host_set = &ts->sigsuspend_mask;
+    target_sigset_t *target_sigset;
+
+    if (sigsize != sizeof(*target_sigset)) {
+        /* Like the kernel, we enforce correct size sigsets */
+        return -TARGET_EINVAL;
+    }
+
+    target_sigset = lock_user(VERIFY_READ, sigset, sigsize, 1);
+    if (!target_sigset) {
+        return -TARGET_EFAULT;
+    }
+    target_to_host_sigset(host_set, target_sigset);
+    unlock_user(target_sigset, sigset, 0);
+
+    *pset = host_set;
+    return 0;
+}
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ecd00382a8..154cb1c7e8 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9557,41 +9557,35 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
 #ifdef TARGET_NR_sigsuspend
     case TARGET_NR_sigsuspend:
         {
-            TaskState *ts = cpu->opaque;
+            sigset_t *set;
+
 #if defined(TARGET_ALPHA)
+            TaskState *ts = cpu->opaque;
             /* target_to_host_old_sigset will bswap back */
             abi_ulong mask = tswapal(arg1);
-            target_to_host_old_sigset(&ts->sigsuspend_mask, &mask);
+            set = &ts->sigsuspend_mask;
+            target_to_host_old_sigset(set, &mask);
 #else
-            if (!(p = lock_user(VERIFY_READ, arg1, sizeof(target_sigset_t), 1)))
-                return -TARGET_EFAULT;
-            target_to_host_old_sigset(&ts->sigsuspend_mask, p);
-            unlock_user(p, arg1, 0);
-#endif
-            ret = get_errno(safe_rt_sigsuspend(&ts->sigsuspend_mask,
-                                               SIGSET_T_SIZE));
-            if (ret != -QEMU_ERESTARTSYS) {
-                ts->in_sigsuspend = 1;
+            ret = process_sigsuspend_mask(&set, arg1, sizeof(target_sigset_t));
+            if (ret != 0) {
+                return ret;
             }
+#endif
+            ret = get_errno(safe_rt_sigsuspend(set, SIGSET_T_SIZE));
+            finish_sigsuspend_mask(ret);
         }
         return ret;
 #endif
     case TARGET_NR_rt_sigsuspend:
         {
-            TaskState *ts = cpu->opaque;
+            sigset_t *set;
 
-            if (arg2 != sizeof(target_sigset_t)) {
-                return -TARGET_EINVAL;
-            }
-            if (!(p = lock_user(VERIFY_READ, arg1, sizeof(target_sigset_t), 1)))
-                return -TARGET_EFAULT;
-            target_to_host_sigset(&ts->sigsuspend_mask, p);
-            unlock_user(p, arg1, 0);
-            ret = get_errno(safe_rt_sigsuspend(&ts->sigsuspend_mask,
-                                               SIGSET_T_SIZE));
-            if (ret != -QEMU_ERESTARTSYS) {
-                ts->in_sigsuspend = 1;
+            ret = process_sigsuspend_mask(&set, arg1, arg2);
+            if (ret != 0) {
+                return ret;
             }
+            ret = get_errno(safe_rt_sigsuspend(set, SIGSET_T_SIZE));
+            finish_sigsuspend_mask(ret);
         }
         return ret;
 #ifdef TARGET_NR_rt_sigtimedwait
-- 
2.25.1

Re: [PATCH 2/5] linux-user: Split out helpers for sigsuspend

[Laurent Vivier]

Le 15/03/2022 à 09:43, Richard Henderson a écrit :
> Two new functions: process_sigsuspend_mask and finish_sigsuspend_mask.
> Move the size check and copy-from-user code.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/signal-common.h | 26 +++++++++++++++++++++++++
>   linux-user/signal.c        | 23 ++++++++++++++++++++++
>   linux-user/syscall.c       | 40 ++++++++++++++++----------------------
>   3 files changed, 66 insertions(+), 23 deletions(-)
> 

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

[PATCH 3/5] linux-user: Properly handle sigset arg to pselect

[Richard Henderson]

Unblocked signals are never delivered, because we
didn't record the new mask for process_pending_signals.
Handle this with the same mechanism as sigsuspend.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/834
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/syscall.c | 30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 154cb1c7e8..8071a5191d 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1391,14 +1391,12 @@ static abi_long do_pselect6(abi_long arg1, abi_long arg2, abi_long arg3,
      * The 6th arg is actually two args smashed together,
      * so we cannot use the C library.
      */
-    sigset_t set;
     struct {
         sigset_t *set;
         size_t size;
     } sig, *sig_ptr;
 
     abi_ulong arg_sigset, arg_sigsize, *arg7;
-    target_sigset_t *target_sigset;
 
     n = arg1;
     rfd_addr = arg2;
@@ -1439,10 +1437,8 @@ static abi_long do_pselect6(abi_long arg1, abi_long arg2, abi_long arg3,
     }
 
     /* Extract the two packed args for the sigset */
+    sig_ptr = NULL;
     if (arg6) {
-        sig_ptr = &sig;
-        sig.size = SIGSET_T_SIZE;
-
         arg7 = lock_user(VERIFY_READ, arg6, sizeof(*arg7) * 2, 1);
         if (!arg7) {
             return -TARGET_EFAULT;
@@ -1452,28 +1448,22 @@ static abi_long do_pselect6(abi_long arg1, abi_long arg2, abi_long arg3,
         unlock_user(arg7, arg6, 0);
 
         if (arg_sigset) {
-            sig.set = &set;
-            if (arg_sigsize != sizeof(*target_sigset)) {
-                /* Like the kernel, we enforce correct size sigsets */
-                return -TARGET_EINVAL;
+            ret = process_sigsuspend_mask(&sig.set, arg_sigset, arg_sigsize);
+            if (ret != 0) {
+                return ret;
             }
-            target_sigset = lock_user(VERIFY_READ, arg_sigset,
-                                      sizeof(*target_sigset), 1);
-            if (!target_sigset) {
-                return -TARGET_EFAULT;
-            }
-            target_to_host_sigset(&set, target_sigset);
-            unlock_user(target_sigset, arg_sigset, 0);
-        } else {
-            sig.set = NULL;
+            sig_ptr = &sig;
+            sig.size = SIGSET_T_SIZE;
         }
-    } else {
-        sig_ptr = NULL;
     }
 
     ret = get_errno(safe_pselect6(n, rfds_ptr, wfds_ptr, efds_ptr,
                                   ts_ptr, sig_ptr));
 
+    if (sig_ptr) {
+        finish_sigsuspend_mask(ret);
+    }
+
     if (!is_error(ret)) {
         if (rfd_addr && copy_to_user_fdset(rfd_addr, &rfds, n)) {
             return -TARGET_EFAULT;
-- 
2.25.1

Re: [PATCH 3/5] linux-user: Properly handle sigset arg to pselect

[Laurent Vivier]

Le 15/03/2022 à 09:43, Richard Henderson a écrit :
> Unblocked signals are never delivered, because we
> didn't record the new mask for process_pending_signals.
> Handle this with the same mechanism as sigsuspend.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/834
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/syscall.c | 30 ++++++++++--------------------
>   1 file changed, 10 insertions(+), 20 deletions(-)
> 

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

[PATCH 4/5] linux-user: Properly handle sigset arg to epoll_pwait

[Richard Henderson]

Unblocked signals are never delivered, because we
didn't record the new mask for process_pending_signals.
Handle this with the same mechanism as sigsuspend.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/syscall.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 8071a5191d..85de4e1bc7 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -12694,29 +12694,21 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
 #if defined(TARGET_NR_epoll_pwait)
         case TARGET_NR_epoll_pwait:
         {
-            target_sigset_t *target_set;
-            sigset_t _set, *set = &_set;
+            sigset_t *set = NULL;
 
             if (arg5) {
-                if (arg6 != sizeof(target_sigset_t)) {
-                    ret = -TARGET_EINVAL;
+                ret = process_sigsuspend_mask(&set, arg5, arg6);
+                if (ret != 0) {
                     break;
                 }
-
-                target_set = lock_user(VERIFY_READ, arg5,
-                                       sizeof(target_sigset_t), 1);
-                if (!target_set) {
-                    ret = -TARGET_EFAULT;
-                    break;
-                }
-                target_to_host_sigset(set, target_set);
-                unlock_user(target_set, arg5, 0);
-            } else {
-                set = NULL;
             }
 
             ret = get_errno(safe_epoll_pwait(epfd, ep, maxevents, timeout,
                                              set, SIGSET_T_SIZE));
+
+            if (set) {
+                finish_sigsuspend_mask(ret);
+            }
             break;
         }
 #endif
-- 
2.25.1

Re: [PATCH 4/5] linux-user: Properly handle sigset arg to epoll_pwait

[Laurent Vivier]

Le 15/03/2022 à 09:43, Richard Henderson a écrit :
> Unblocked signals are never delivered, because we
> didn't record the new mask for process_pending_signals.
> Handle this with the same mechanism as sigsuspend.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/syscall.c | 22 +++++++---------------
>   1 file changed, 7 insertions(+), 15 deletions(-)
> 

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

[PATCH 5/5] linux-user: Properly handle sigset arg to ppoll

[Richard Henderson]

Unblocked signals are never delivered, because we
didn't record the new mask for process_pending_signals.
Handle this with the same mechanism as sigsuspend.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/syscall.c | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 85de4e1bc7..a69b7084f1 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1519,8 +1519,7 @@ static abi_long do_ppoll(abi_long arg1, abi_long arg2, abi_long arg3,
     }
     if (ppoll) {
         struct timespec _timeout_ts, *timeout_ts = &_timeout_ts;
-        target_sigset_t *target_set;
-        sigset_t _set, *set = &_set;
+        sigset_t *set = NULL;
 
         if (arg3) {
             if (time64) {
@@ -1539,25 +1538,19 @@ static abi_long do_ppoll(abi_long arg1, abi_long arg2, abi_long arg3,
         }
 
         if (arg4) {
-            if (arg5 != sizeof(target_sigset_t)) {
+            ret = process_sigsuspend_mask(&set, arg4, arg5);
+            if (ret != 0) {
                 unlock_user(target_pfd, arg1, 0);
-                return -TARGET_EINVAL;
+                return ret;
             }
-
-            target_set = lock_user(VERIFY_READ, arg4,
-                                   sizeof(target_sigset_t), 1);
-            if (!target_set) {
-                unlock_user(target_pfd, arg1, 0);
-                return -TARGET_EFAULT;
-            }
-            target_to_host_sigset(set, target_set);
-        } else {
-            set = NULL;
         }
 
         ret = get_errno(safe_ppoll(pfd, nfds, timeout_ts,
                                    set, SIGSET_T_SIZE));
 
+        if (set) {
+            finish_sigsuspend_mask(ret);
+        }
         if (!is_error(ret) && arg3) {
             if (time64) {
                 if (host_to_target_timespec64(arg3, timeout_ts)) {
@@ -1569,9 +1562,6 @@ static abi_long do_ppoll(abi_long arg1, abi_long arg2, abi_long arg3,
                 }
             }
         }
-        if (arg4) {
-            unlock_user(target_set, arg4, 0);
-        }
     } else {
           struct timespec ts, *pts;
 
-- 
2.25.1

Re: [PATCH 5/5] linux-user: Properly handle sigset arg to ppoll

[Laurent Vivier]

Le 15/03/2022 à 09:43, Richard Henderson a écrit :
> Unblocked signals are never delivered, because we
> didn't record the new mask for process_pending_signals.
> Handle this with the same mechanism as sigsuspend.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/syscall.c | 24 +++++++-----------------
>   1 file changed, 7 insertions(+), 17 deletions(-)
> 

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

Re: [PATCH 0/5] linux-user: signal mask fixes for pselect et al

[Laurent Vivier]

Le 15/03/2022 à 09:43, Richard Henderson a écrit :
> Split out some helpers from sigsuspend.
> Reuse them for pselect, epoll_pwait, ppoll.
> Fix an alpha buglet found on the way.
> 
> r~
> 
> Richard Henderson (5):
>    linux-user/alpha: Fix sigsuspend for big-endian hosts
>    linux-user: Split out helpers for sigsuspend
>    linux-user: Properly handle sigset arg to pselect
>    linux-user: Properly handle sigset arg to epoll_pwait
>    linux-user: Properly handle sigset arg to ppoll
> 
>   linux-user/signal-common.h |  26 ++++++++
>   linux-user/signal.c        |  23 +++++++
>   linux-user/syscall.c       | 119 ++++++++++++++-----------------------
>   3 files changed, 92 insertions(+), 76 deletions(-)
> 

Series applied to my linux-user-for-7.0 branch.

Thanks,
Laurent