When called with WNOHANG and no child has exited, waitid returns with
info.si_pid set to zero and thus check for info.si_pid != 0 will cause
target siginfo structure to be uninitialized.  Fixed by removing the check.

Signed-off-by: Serge Belyshev <belyshev@depni.sinp.msu.ru>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/817
---
 linux-user/syscall.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 5950222a77..b80531ac4c 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8724,9 +8724,8 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
     case TARGET_NR_waitid:
         {
             siginfo_t info;
-            info.si_pid = 0;
             ret = get_errno(safe_waitid(arg1, arg2, &info, arg4, NULL));
-            if (!is_error(ret) && arg3 && info.si_pid != 0) {
+            if (!is_error(ret) && arg3) {
                 if (!(p = lock_user(VERIFY_WRITE, arg3, sizeof(target_siginfo_t), 0)))
                     return -TARGET_EFAULT;
                 host_to_target_siginfo(p, &info);
-- 
2.34.1

Le 13/01/2022 à 10:37, Serge Belyshev a écrit :
> When called with WNOHANG and no child has exited, waitid returns with
> info.si_pid set to zero and thus check for info.si_pid != 0 will cause
> target siginfo structure to be uninitialized.  Fixed by removing the check.
> 
> Signed-off-by: Serge Belyshev <belyshev@depni.sinp.msu.ru>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/817
> ---
>   linux-user/syscall.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 5950222a77..b80531ac4c 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -8724,9 +8724,8 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>       case TARGET_NR_waitid:
>           {
>               siginfo_t info;
> -            info.si_pid = 0;
>               ret = get_errno(safe_waitid(arg1, arg2, &info, arg4, NULL));
> -            if (!is_error(ret) && arg3 && info.si_pid != 0) {
> +            if (!is_error(ret) && arg3) {
>                   if (!(p = lock_user(VERIFY_WRITE, arg3, sizeof(target_siginfo_t), 0)))
>                       return -TARGET_EFAULT;
>                   host_to_target_siginfo(p, &info);

According to wait(2), it sounds a little bit more complicated than that.

        If WNOHANG was specified in options and there were no children in a waitable state, then
        waitid() returns 0 immediately and the state of the siginfo_t  structure  pointed  to  by
        infop  depends  on  the  implementation.   To (portably) distinguish this case from that
        where a child was in a waitable state, zero out the si_pid field before the call and check
        for a nonzero value in this field after the call returns.

        POSIX.1-2008  Technical  Corrigendum  1 (2013) adds the requirement that when WNOHANG is
        specified in options and there were no children in a waitable state, then waitid() should
        zero out the si_pid and si_signo fields of the structure.  On Linux and other implementations
        that adhere to this requirement, it is not necessary to zero out the si_pid field before
        calling waitid().  However, not all implementations follow the POSIX.1  specification  on
        this point.

Perhaps the best approach would be to copy the caller target siginfo to the host one, call host 
waitpid(), remove the "info.si_pid != 0" and copy back the host siginfo to target one?

Thanks,
Laurent

Laurent Vivier <laurent@vivier.eu> writes:

> ...
>
> According to wait(2), it sounds a little bit more complicated than that.
>
>        If WNOHANG was specified in options and there were no children in a waitable state, then
>        waitid() returns 0 immediately and the state of the siginfo_t  structure  pointed  to  by
>        infop  depends  on  the  implementation.   To (portably) distinguish this case from that
>        where a child was in a waitable state, zero out the si_pid field before the call and check
>        for a nonzero value in this field after the call returns.
>
>        POSIX.1-2008  Technical  Corrigendum  1 (2013) adds the requirement that when WNOHANG is
>        specified in options and there were no children in a waitable state, then waitid() should
>        zero out the si_pid and si_signo fields of the structure.  On Linux and other implementations
>        that adhere to this requirement, it is not necessary to zero out the si_pid field before
>        calling waitid().  However, not all implementations follow the POSIX.1  specification  on
>        this point.
>

In glibc waitpid is implemented using wait4, and on systems where wait4
is not available (e.g. riscv32), wait4 is implemented via waitid and the
implementation expects that info.si_pid is cleared when appropriate:

(from https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/wait4.c#l58 )

  siginfo_t infop;
  ...
  SYSCALL_CANCEL (waitid, idtype, pid, &infop, options, usage)
  ...
  return infop.si_pid;

so I think it is safe to follow glibc here and rely on the kernel to clear
pid/signo and other fields and just to remove the "info.si_pid != 0" check.


> Perhaps the best approach would be to copy the caller target siginfo
> to the host one, call host waitpid(), remove the "info.si_pid != 0"
> and copy back the host siginfo to target one?

Not sure what would be the gain in this case, as linux clears siginfo fiels
since the very first implementation of waitid in 2.6.9:

https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/tree/kernel/exit.c?h=v2.6.9#n1354

	/*
	 * For a WNOHANG return, clear out all the fields
	 * we would set so the user can easily tell the
	 * difference.
	 */