Re: [Qemu-devel] [PATCH] linux-user: fix getrusage and wait4 failures with invalid rusage struct

[Petar Jovanovic]

________________________________________
From: Andreas Färber address@hidden
Sent: Monday, May 05, 2014 11:55 AM
To: Petar Jovanovic; address@hidden
Cc: address@hidden; Petar Jovanovic
Subject: Re: [Qemu-devel] [PATCH] linux-user: fix getrusage and wait4 failures 
with invalid rusage struct

Am 08.04.2014 19:24, schrieb Petar Jovanovic:
> From: Petar Jovanovic <address@hidden>
>
> Implementations of system calls getrusage and wait4 have not previously
> handled correctly cases when incorrect address of struct rusage is
> passed.
> This change makes sure return values are correctly set for these cases.
>
> Signed-off-by: Petar Jovanovic <address@hidden>
> ---
>  linux-user/syscall.c |   11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 9864813..fc52f0b 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -6309,7 +6309,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>              struct rusage rusage;
>              ret = get_errno(getrusage(arg1, &rusage));
>              if (!is_error(ret)) {
> -                host_to_target_rusage(arg2, &rusage);
> +                ret = host_to_target_rusage(arg2, &rusage);
>              }
>          }
>          break;

> Why do you always set ret here ...

> @@ -6974,6 +6974,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>              abi_long status_ptr = arg2;
>              struct rusage rusage, *rusage_ptr;
>              abi_ulong target_rusage = arg4;
> +            abi_long rusage_err;
>              if (target_rusage)
>                  rusage_ptr = &rusage;
>              else
> @@ -6985,8 +6986,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>                      if (put_user_s32(status, status_ptr))
>                          goto efault;
>                  }
> -                if (target_rusage)
> -                    host_to_target_rusage(target_rusage, &rusage);
> +                if (target_rusage) {
> +                    rusage_err = host_to_target_rusage(target_rusage, 
> &rusage);
> +                    if (rusage_err) {
> +                        ret = rusage_err;
> +                    }
> +                }
>              }
>          }
>          break;

> ... but only on error here? Isn't obvious from the commit message.

This is due to the fact that on success, wait4() returns the process ID
of the child whose state has changed. So, the code needs to take care
not to override it.

Regards,
Petar