linux-kernel - Re: [PATCH] compat: fix use of an uninitialized variable in compat_sys_io

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <AANLkTinL8Zi2=XjRrb1g7Wg=tqGk3Tmk3C7-CzZY_QjJ@mail.gmail.com>
Date:	Mon, 3 Jan 2011 13:46:19 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Namhyung Kim <namhyung@...il.com>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	Jeff Moyer <jmoyer@...hat.com>
Subject: Re: [PATCH] compat: fix use of an uninitialized variable in compat_sys_io_setup()

On Mon, Jan 3, 2011 at 1:19 PM, Andrew Morton <akpm@...ux-foundation.org> wrote:
> On Fri, 24 Dec 2010 18:16:26 +0900
> Namhyung Kim <namhyung@...il.com> wrote:
>
>> The upper bytes of ctx64 might contain garbages because it was
>> set by get_user() which copied only lower 4 bytes as its second
>> argument points to. Since sys_io_setup() requires its argumet
>> is properly initialized to 0 we should set it explicitly.
>>
>> On x86, this was not a problem since its implementation of
>> get_user() does a C assignment so that it can fill upper bytes
>> with 0's. But other archs that use __get_user_asm() or something
>> might have a problem.
>>
>> Signed-off-by: Namhyung Kim <namhyung@...il.com>
>> Cc: Jeff Moyer <jmoyer@...hat.com>
>> ---
>>  fs/compat.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/compat.c b/fs/compat.c
>> index 4376e07febbb..b074e9f79148 100644
>> --- a/fs/compat.c
>> +++ b/fs/compat.c
>> @@ -526,7 +526,7 @@ asmlinkage long
>>  compat_sys_io_setup(unsigned nr_reqs, u32 __user *ctx32p)
>>  {
>>       long ret;
>> -     aio_context_t ctx64;
>> +     aio_context_t ctx64 = 0;
>>
>>       mm_segment_t oldfs = get_fs();
>>       if (unlikely(get_user(ctx64, ctx32p)))
>
> Well.  What _should_ a get_user(some_u64, some_u32*) do to `some_u64'?

We do "get_user(int,char *)" etc all the time. It's expected to work,
and do the proper zero- (or sign-) extension.

The x86 code does:

  #define get_user(x, ptr)
           ...
           (x) = (__typeof__(*(ptr)))__val_gu;

and that's the _only_ correct thing to do. That assignment, with the
proper type expansion, is absolutely vital. And there's no way you can
do that with any asm constructs, exactly because of the whole
sign/zero extension requirement.

If somebody only sets the low 32 bits of the result, that's simply a
bug. That's a seriously buggered uaccess.h.

So NAK on the patch, and please point to what architecture has this problem.

                            Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/