linux-kernel - Re: [PATCH 1/4] kasan: support alloca() poisoning

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACT4Y+Yo1gKVJQF74kG5gZ60Qmzo65=6NLnN69ybd+QtzfAi1w@mail.gmail.com>
Date:   Fri, 14 Jul 2017 08:13:33 +0200
From:   Dmitry Vyukov <dvyukov@...gle.com>
To:     Greg Hackmann <ghackmann@...gle.com>
Cc:     Andrey Ryabinin <aryabinin@...tuozzo.com>,
        Alexander Potapenko <glider@...gle.com>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Michal Marek <mmarek@...e.com>,
        LKML <linux-kernel@...r.kernel.org>,
        kasan-dev <kasan-dev@...glegroups.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "open list:KERNEL BUILD + fi..." <linux-kbuild@...r.kernel.org>,
        Matthias Kaehlcke <mka@...omium.org>,
        Michael Davidson <md@...gle.com>
Subject: Re: [PATCH 1/4] kasan: support alloca() poisoning

On Fri, Jul 14, 2017 at 12:40 AM, Greg Hackmann <ghackmann@...gle.com> wrote:
> Hi,
>
> Thanks for taking a look at this patchstack.  I apologize for the delay in
> responding.
>
> On 07/10/2017 01:44 AM, Dmitry Vyukov wrote:
>>>
>>> +
>>> +       const void *left_redzone = (const void *)(addr -
>>> +                       KASAN_ALLOCA_REDZONE_SIZE);
>>> +       const void *right_redzone = (const void *)(addr +
>>> rounded_up_size);
>>
>>
>> Please check that size is rounded to KASAN_ALLOCA_REDZONE_SIZE. That's
>> the expectation, right? That can change is clang silently.
>>
>>> +       kasan_poison_shadow(left_redzone, KASAN_ALLOCA_REDZONE_SIZE,
>>> +                       KASAN_ALLOCA_LEFT);
>>> +       kasan_poison_shadow(right_redzone,
>>> +                       padding_size + KASAN_ALLOCA_REDZONE_SIZE,
>>> +                       KASAN_ALLOCA_RIGHT);
>>
>>
>> We also need to poison the unaligned part at the end of the object
>> from size to rounded_up_size. You can see how we do it for heap
>> objects.
>
>
> The expectation is that `size' is the exact size of the alloca()ed object.
> `rounded_up_size' then adds the 0-7 bytes needed to adjust the size to the
> ASAN shadow scale.  So `addr + rounded_up_size' should be the correct place
> to start poisoning.


We need to start poisoning at addr+size exactly.
Asan shadow scheme supports this. It's not possible to poison
beginning of an aligned 8-byte block, but leave tail unpoisoned. But
it is possible to poison tail of an aligned 8-byte block and leave
beginning unpoisoned. Look at what we do for kmalloc.


> In retrospect this part of the code was pretty confusing.  How about this?
> I think its intent is clearer, plus it's a closer match for the description
> in my commit message:
>
>         unsigned long left_redzone_start;
>         unsigned long object_end;
>         unsigned long right_redzone_start, right_redzone_end;
>
>         left_redzone_start = addr - KASAN_ALLOCA_REDZONE_SIZE;
>         kasan_poison_shadow((const void *)left_redzone_start,
>                         KASAN_ALLOCA_REDZONE_SIZE,
>                         KASAN_ALLOCA_LEFT);
>
>         object_end = round_up(addr + size, KASAN_SHADOW_SCALE_SIZE);
>         right_redzone_start = round_up(object_end,
> KASAN_ALLOCA_REDZONE_SIZE);
>         right_redzone_end = right_redzone_start + KASAN_ALLOCA_REDZONE_SIZE;
>         kasan_poison_shadow((const void *)object_end,
>                         right_redzone_end - object_end,
>                         KASAN_ALLOCA_RIGHT);