qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] linux-user: fix getrusage and wait4 failures with invalid rusage struct
@ 2014-04-08 17:24 Petar Jovanovic
  2014-05-05  9:02 ` Petar Jovanovic
  2014-05-05  9:55 ` Andreas Färber
  0 siblings, 2 replies; 6+ messages in thread
From: Petar Jovanovic @ 2014-04-08 17:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: riku.voipio, petar.jovanovic

From: Petar Jovanovic <petar.jovanovic@imgtec.com>

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 <petar.jovanovic@imgtec.com>
---
 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;
@@ -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;
-- 
1.7.9.5

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

* Re: [Qemu-devel] [PATCH] linux-user: fix getrusage and wait4 failures with invalid rusage struct
  2014-04-08 17:24 [Qemu-devel] [PATCH] linux-user: fix getrusage and wait4 failures with invalid rusage struct Petar Jovanovic
@ 2014-05-05  9:02 ` Petar Jovanovic
  2014-05-05 12:27   ` Riku Voipio
  2014-05-05  9:55 ` Andreas Färber
  1 sibling, 1 reply; 6+ messages in thread
From: Petar Jovanovic @ 2014-05-05  9:02 UTC (permalink / raw)
  To: Petar Jovanovic, qemu-devel@nongnu.org; +Cc: riku.voipio@linaro.org

ping

http://patchwork.ozlabs.org/patch/337703/

________________________________________
From: Petar Jovanovic [petar.jovanovic@rt-rk.com]
Sent: Tuesday, April 08, 2014 7:24 PM
To: qemu-devel@nongnu.org
Cc: Petar Jovanovic; riku.voipio@linaro.org
Subject: [PATCH] linux-user: fix getrusage and wait4 failures with invalid rusage struct

From: Petar Jovanovic <petar.jovanovic@imgtec.com>

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 <petar.jovanovic@imgtec.com>
---
 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;
@@ -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;
--
1.7.9.5


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

* Re: [Qemu-devel] [PATCH] linux-user: fix getrusage and wait4 failures with invalid rusage struct
  2014-04-08 17:24 [Qemu-devel] [PATCH] linux-user: fix getrusage and wait4 failures with invalid rusage struct Petar Jovanovic
  2014-05-05  9:02 ` Petar Jovanovic
@ 2014-05-05  9:55 ` Andreas Färber
  2014-05-05 10:12   ` Petar Jovanovic
  1 sibling, 1 reply; 6+ messages in thread
From: Andreas Färber @ 2014-05-05  9:55 UTC (permalink / raw)
  To: Petar Jovanovic, qemu-devel; +Cc: riku.voipio, petar.jovanovic

Am 08.04.2014 19:24, schrieb Petar Jovanovic:
> From: Petar Jovanovic <petar.jovanovic@imgtec.com>
> 
> 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 <petar.jovanovic@imgtec.com>
> ---
>  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.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

* Re: [Qemu-devel] [PATCH] linux-user: fix getrusage and wait4 failures with invalid rusage struct
  2014-05-05  9:55 ` Andreas Färber
@ 2014-05-05 10:12   ` Petar Jovanovic
  0 siblings, 0 replies; 6+ messages in thread
From: Petar Jovanovic @ 2014-05-05 10:12 UTC (permalink / raw)
  To: Andreas Färber, Petar Jovanovic, qemu-devel@nongnu.org
  Cc: riku.voipio@linaro.org


________________________________________
From: Andreas Färber [afaerber@suse.de]
Sent: Monday, May 05, 2014 11:55 AM
To: Petar Jovanovic; qemu-devel@nongnu.org
Cc: riku.voipio@linaro.org; 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 <petar.jovanovic@imgtec.com>
>
> 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 <petar.jovanovic@imgtec.com>
> ---
>  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


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

* Re: [Qemu-devel] [PATCH] linux-user: fix getrusage and wait4 failures with invalid rusage struct
  2014-05-05  9:02 ` Petar Jovanovic
@ 2014-05-05 12:27   ` Riku Voipio
  2014-05-05 12:29     ` Peter Maydell
  0 siblings, 1 reply; 6+ messages in thread
From: Riku Voipio @ 2014-05-05 12:27 UTC (permalink / raw)
  To: Petar Jovanovic, Peter Maydell; +Cc: Petar Jovanovic, qemu-devel@nongnu.org

[-- Attachment #1: Type: text/plain, Size: 2741 bytes --]

Hi,

Thanks, looks good and fixes getrusage02 ltp test.

Added to the linux-user que:

https://git.linaro.org/people/riku.voipio/qemu.git/shortlog/refs/heads/linux-user-for-upstream

Peter, do you prefer a new pull request for linux-user with this patch
added on top, or is pulling the updated branch good enough for you?

Riku


On 5 May 2014 12:02, Petar Jovanovic <Petar.Jovanovic@imgtec.com> wrote:

> ping
>
> http://patchwork.ozlabs.org/patch/337703/
>
> ________________________________________
> From: Petar Jovanovic [petar.jovanovic@rt-rk.com]
> Sent: Tuesday, April 08, 2014 7:24 PM
> To: qemu-devel@nongnu.org
> Cc: Petar Jovanovic; riku.voipio@linaro.org
> Subject: [PATCH] linux-user: fix getrusage and wait4 failures with invalid
> rusage struct
>
> From: Petar Jovanovic <petar.jovanovic@imgtec.com>
>
> 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 <petar.jovanovic@imgtec.com>
> ---
>  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;
> @@ -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;
> --
> 1.7.9.5
>
>

[-- Attachment #2: Type: text/html, Size: 4084 bytes --]

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

* Re: [Qemu-devel] [PATCH] linux-user: fix getrusage and wait4 failures with invalid rusage struct
  2014-05-05 12:27   ` Riku Voipio
@ 2014-05-05 12:29     ` Peter Maydell
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2014-05-05 12:29 UTC (permalink / raw)
  To: Riku Voipio; +Cc: qemu-devel@nongnu.org, Petar Jovanovic, Petar Jovanovic

On 5 May 2014 13:27, Riku Voipio <riku.voipio@linaro.org> wrote:
> Hi,
>
> Thanks, looks good and fixes getrusage02 ltp test.
>
> Added to the linux-user que:
>
> https://git.linaro.org/people/riku.voipio/qemu.git/shortlog/refs/heads/linux-user-for-upstream
>
> Peter, do you prefer a new pull request for linux-user with this patch added
> on top, or is pulling the updated branch good enough for you?

Send a new cover letter email, then it will go into my folder
of pending requests correctly.

thanks
-- PMM

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

end of thread, other threads:[~2014-05-05 12:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-08 17:24 [Qemu-devel] [PATCH] linux-user: fix getrusage and wait4 failures with invalid rusage struct Petar Jovanovic
2014-05-05  9:02 ` Petar Jovanovic
2014-05-05 12:27   ` Riku Voipio
2014-05-05 12:29     ` Peter Maydell
2014-05-05  9:55 ` Andreas Färber
2014-05-05 10:12   ` Petar Jovanovic

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).