[Qemu-devel] [PATCH] Fix buffer overrun in sched_getaffinity

[Qemu-devel] [PATCH] Fix buffer overrun in sched_getaffinity

[Mike McCormack]

Zeroing of the cpu array should start from &cpus[kernel_ret]
not &cpus[num_zeros_to_fill].

This fixes a crash in EFL's edje_cc running under qemu-arm.

Signed-off-by: Mike McCormack <mj.mccormack@samsung.com>
---
 linux-user/syscall.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index bb0999d..1cda10a 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -6389,7 +6389,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
                     unsigned long zero = arg2 - ret;
                     p = alloca(zero);
                     memset(p, 0, zero);
-                    if (copy_to_user(arg3 + zero, p, zero)) {
+                    if (copy_to_user(arg3 + ret, p, zero)) {
                         goto efault;
                     }
                     arg2 = ret;
-- 
1.7.0.4

Re: [Qemu-devel] [PATCH] Fix buffer overrun in sched_getaffinity

[Stefan Hajnoczi]

On Tue, Apr 12, 2011 at 12:48:20PM +0900, Mike McCormack wrote:
> Zeroing of the cpu array should start from &cpus[kernel_ret]
> not &cpus[num_zeros_to_fill].
> 
> This fixes a crash in EFL's edje_cc running under qemu-arm.
> 
> Signed-off-by: Mike McCormack <mj.mccormack@samsung.com>
> ---
>  linux-user/syscall.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index bb0999d..1cda10a 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -6389,7 +6389,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>                      unsigned long zero = arg2 - ret;
>                      p = alloca(zero);
>                      memset(p, 0, zero);
> -                    if (copy_to_user(arg3 + zero, p, zero)) {
> +                    if (copy_to_user(arg3 + ret, p, zero)) {
>                          goto efault;
>                      }
>                      arg2 = ret;

Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Note that mainline Linux does not zero unwritten bytes.  I would drop
the entire arg2 > ret case and instead copy only ret bytes to user.

Stefan

Re: [Qemu-devel] [PATCH] Fix buffer overrun in sched_getaffinity

[Mike Frysinger]

On Sat, Apr 16, 2011 at 05:16, Stefan Hajnoczi wrote:
> On Tue, Apr 12, 2011 at 12:48:20PM +0900, Mike McCormack wrote:
>> Zeroing of the cpu array should start from &cpus[kernel_ret]
>> not &cpus[num_zeros_to_fill].
>>
>> This fixes a crash in EFL's edje_cc running under qemu-arm.
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index bb0999d..1cda10a 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -6389,7 +6389,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>                      unsigned long zero = arg2 - ret;
>>                      p = alloca(zero);
>>                      memset(p, 0, zero);
>> -                    if (copy_to_user(arg3 + zero, p, zero)) {
>> +                    if (copy_to_user(arg3 + ret, p, zero)) {
>>                          goto efault;
>>                      }
>>                      arg2 = ret;
>
> Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>
> Note that mainline Linux does not zero unwritten bytes.  I would drop
> the entire arg2 > ret case and instead copy only ret bytes to user.

both changes make sense to me
Acked-by: Mike Frysinger <vapier@gentoo.org>
-mike