linux-kernel - Re: [PATCH] fs/proc/kcore.c: Omit kernel text area for hardened usercopy feature

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CA+55aFzpVnhpfG6p9Y8JGo1+__mcBtX5H2ZFv5kmtacMtK5BiQ@mail.gmail.com>
Date:   Wed, 7 Sep 2016 09:58:01 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Andi Kleen <andi@...stfloor.org>
Cc:     Jiri Olsa <jolsa@...hat.com>, Kees Cook <keescook@...omium.org>,
        Jiri Olsa <jolsa@...nel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Adrian Hunter <adrian.hunter@...el.com>,
        KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
Subject: Re: [PATCH] fs/proc/kcore.c: Omit kernel text area for hardened
 usercopy feature

On Wed, Sep 7, 2016 at 9:38 AM, Andi Kleen <andi@...stfloor.org> wrote:
>>
>> -                             n = copy_to_user(buffer, (char *)start, tsz);
>> +                             buf = kzalloc(tsz, GFP_KERNEL);
>
> You have to add some limit and a loop, otherwise a user can eat all kernel memory,
> or copies > KMALLOC_MAX wouldn't work. Probably only get a single page.

'start' and 'tsz' is already chunked to be aligned pages (well, as
aligned as they can be: the beginning and end obviously won't be).
Above the loop:

        if ((tsz = (PAGE_SIZE - (start & ~PAGE_MASK))) > buflen)
                tsz = buflen;

and then inside the loop:

                tsz = (buflen > PAGE_SIZE ? PAGE_SIZE : buflen);

so it's already limited to one page.

That said, it *might* be worth moving the temporary allocation to the
top, or even to move it to open_kcore(). It used to be a special case
for just the vmalloc region, now it's always done.

So instead of having two different copies of the same special case for
the two different cases, why not try to unify them and just have one
common (page-sized) buffer allocation?

                 Linus