Patchwork Fix buffer overrun in sched_getaffinity

login
register
mail settings
Submitter Mike McCormack
Date April 12, 2011, 3:48 a.m.
Message ID <4DA3CB84.5080503@samsung.com>
Download mbox | patch
Permalink /patch/90698/
State New
Headers show

Comments

Mike McCormack - April 12, 2011, 3:48 a.m.
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(-)
Stefan Hajnoczi - April 16, 2011, 9:16 a.m.
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
Mike Frysinger - April 16, 2011, 6:18 p.m.
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

Patch

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;