Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1341038 > unrolled thread

Re: [PATCHv2] lkdtm: Add READ_AFTER_FREE test

Started byKees Cook <keescook@chromium.org>
First post2016-02-23 22:30 +0100
Last post2016-03-01 02:40 +0100
Articles 14 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCHv2] lkdtm: Add READ_AFTER_FREE test Kees Cook <keescook@chromium.org> - 2016-02-23 22:30 +0100
    Re: [PATCHv2] lkdtm: Add READ_AFTER_FREE test Kees Cook <keescook@chromium.org> - 2016-02-23 23:40 +0100
      Re: [PATCHv2] lkdtm: Add READ_AFTER_FREE test Laura Abbott <labbott@redhat.com> - 2016-02-24 20:00 +0100
    Re: [PATCHv2] lkdtm: Add READ_AFTER_FREE test Kees Cook <keescook@chromium.org> - 2016-02-24 18:30 +0100
      Re: [PATCHv2] lkdtm: Add READ_AFTER_FREE test Laura Abbott <labbott@redhat.com> - 2016-02-24 20:50 +0100
        Re: [PATCHv2] lkdtm: Add READ_AFTER_FREE test Kees Cook <keescook@chromium.org> - 2016-02-24 22:50 +0100
          Re: [PATCHv2] lkdtm: Add READ_AFTER_FREE test Kees Cook <keescook@chromium.org> - 2016-02-25 00:40 +0100
            Re: [PATCHv2] lkdtm: Add READ_AFTER_FREE test Laura Abbott <labbott@redhat.com> - 2016-02-25 02:30 +0100
              Re: [PATCHv2] lkdtm: Add READ_AFTER_FREE test Kees Cook <keescook@chromium.org> - 2016-02-25 18:40 +0100
                Re: [PATCHv2] lkdtm: Add READ_AFTER_FREE test Laura Abbott <labbott@redhat.com> - 2016-02-26 00:20 +0100
                  Re: [PATCHv2] lkdtm: Add READ_AFTER_FREE test Kees Cook <keescook@chromium.org> - 2016-02-26 17:10 +0100
                    Re: [PATCHv2] lkdtm: Add READ_AFTER_FREE test Laura Abbott <labbott@redhat.com> - 2016-02-26 23:20 +0100
                      Re: [PATCHv2] lkdtm: Add READ_AFTER_FREE test Kees Cook <keescook@chromium.org> - 2016-02-26 23:40 +0100
                        Re: [PATCHv2] lkdtm: Add READ_AFTER_FREE test Laura Abbott <labbott@redhat.com> - 2016-03-01 02:40 +0100

#1341038 — Re: [PATCHv2] lkdtm: Add READ_AFTER_FREE test

FromKees Cook <keescook@chromium.org>
Date2016-02-23 22:30 +0100
SubjectRe: [PATCHv2] lkdtm: Add READ_AFTER_FREE test
Message-ID<r5sGS-2L7-17@gated-at.bofh.it>
On Mon, Feb 22, 2016 at 2:06 PM, Laura Abbott <labbott@redhat.com> wrote:
> On 02/22/2016 11:27 AM, Kees Cook wrote:
>>
>> On Fri, Feb 19, 2016 at 3:07 PM, Laura Abbott <labbott@redhat.com> wrote:
>>>
>>> On 02/19/2016 02:19 PM, Kees Cook wrote:
>>>>
>>>>
>>>> On Fri, Feb 19, 2016 at 2:11 PM, Laura Abbott <labbott@redhat.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> On 02/19/2016 11:12 AM, Kees Cook wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Thu, Feb 18, 2016 at 5:15 PM, Laura Abbott
>>>>>> <labbott@fedoraproject.org>
>>>>>> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> In a similar manner to WRITE_AFTER_FREE, add a READ_AFTER_FREE
>>>>>>> test to test free poisoning features. Sample output when
>>>>>>> no sanitization is present:
>>>>>>>
>>>>>>> [   22.414170] lkdtm: Performing direct entry READ_AFTER_FREE
>>>>>>> [   22.415124] lkdtm: Value in memory before free: 12345678
>>>>>>> [   22.415900] lkdtm: Attempting to read from freed memory
>>>>>>> [   22.416394] lkdtm: Successfully read value: 12345678
>>>>>>>
>>>>>>> with sanitization:
>>>>>>>
>>>>>>> [   25.874585] lkdtm: Performing direct entry READ_AFTER_FREE
>>>>>>> [   25.875527] lkdtm: Value in memory before free: 12345678
>>>>>>> [   25.876382] lkdtm: Attempting to read from freed memory
>>>>>>> [   25.876900] general protection fault: 0000 [#1] SMP
>>>>>>>
>>>>>>> Signed-off-by: Laura Abbott <labbott@fedoraproject.org>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Excellent! Could you mention in the changelog which CONFIG (or runtime
>>>>>> values) will change the lkdtm test? (I thought there was a poisoning
>>>>>> style that would result in a zero-read instead of a GP?)
>>>>>>
>>>>>
>>>>> There was a zeroing patch in the first draft but given the direction
>>>>> things are going, I don't see it going in. I'll mention the debug
>>>>> options which will show this though.
>>>>
>>>>
>>>>
>>>> Ah! Okay, I was having trouble following what was happening. What's
>>>> the current state of the use-after-free protections you've been
>>>> working on?
>>>
>>>
>>>
>>> Based on discussion, the SL*B maintainers want to use the existing
>>> slab poisoning features instead adding in new hooks. They also don't
>>> want the fast path to be affected at all. This means most of the
>>> actual work there is improving the performance of slub_debug=P. I
>>> sent out patches for some low hanging fruit in SLUB which improved
>>> the performance by a good bit. Those have been Acked and are sitting
>>> in Andrew's tree. The next performance work involves more in depth
>>> tinkering with the SLUB allocator. Apart from just performance, the
>>> other work would be poisoning for caches with ctors in SLUB and
>>> poisoning in SLOB. I could use some help with benchmarking some
>>> actual use cases to see how usable slub_debug=P would be on some
>>> use cases.
>>>
>>> I did sent out patches for the buddy allocator as well. The last
>>
>>
>> This must be where my confusion stems. :) IIUC, the buddy allocator is
>> used within the SL*B logic when splitting/joining regions? Can we add
>> an lkdtm test for this too?
>>
>
> The buddy allocator backs the underlying SL*B logic. Each SL*B allocation
> is typically less than a page so those allocators manage the smaller
> allocations. I was thinking about an LKDTM test for the buddy allocator
> as well. I'll see about adding one. This would be useful for testing
> debug_pagealloc as well.
>
>
>>>
>>> version I sent out didn't get much in the way of feedback except
>>> for some requests for benchmarks on the zeroing. I was planning
>>> on following up on that next week to see if there was any more feedback
>>> and beg for Acks.
>>
>>
>> If you can point me at the current tree, I'd be happy to run some
>> benchmarks.
>>
>
> mmotm should have the patches
> http://git.cmpxchg.org/cgit.cgi/linux-mmotm.git/
> Turn on CONFIG_PAGE_POISONING and set page_poison=on on the command line.

Okay, it looks like the combinations to test are:

default:
DEBUG_PAGEALLOC=n
PAGE_POISONING=n

heavy-duty:
DEBUG_PAGEALLOC=y (ARCH_SUPPORTS_DEBUG_PAGEALLOC=y)
debug_pagealloc=on

random poison only:
DEBUG_PAGEALLOC=n
PAGE_POISONING=y
PAGE_POISONING_NO_SANITY=y
PAGE_POISONING_ZERO=n
page_poison=on

zero poison only:
DEBUG_PAGEALLOC=n
PAGE_POISONING=y
PAGE_POISONING_NO_SANITY=y
PAGE_POISONING_ZERO=y
page_poison=on

random poison with sanity:
DEBUG_PAGEALLOC=n
PAGE_POISONING=y
PAGE_POISONING_NO_SANITY=n
PAGE_POISONING_ZERO=n
page_poison=on

zero poison with sanity:
DEBUG_PAGEALLOC=n
PAGE_POISONING=y
PAGE_POISONING_NO_SANITY=n
PAGE_POISONING_ZERO=y
page_poison=on


-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [next] | [standalone]

#1341142

FromKees Cook <keescook@chromium.org>
Date2016-02-23 23:40 +0100
Message-ID<r5tMC-3DJ-15@gated-at.bofh.it>
In reply to#1341038
On Tue, Feb 23, 2016 at 1:25 PM, Kees Cook <keescook@chromium.org> wrote:

> zero poison only:
> DEBUG_PAGEALLOC=n
> PAGE_POISONING=y
> PAGE_POISONING_NO_SANITY=y
> PAGE_POISONING_ZERO=y
> page_poison=on

This combo (in next-20160223) results in an unusable system. :(

[    1.754183] random: init urandom read with 11 bits of entropy available
[    1.768449] hostname (1171) used greatest stack depth: 11808 bytes left
[    1.787954] BUG: Bad page map in process init  pte:3d656c6f736e6f63
pmd:00020067
[    1.789177] addr:00007f9f68200000 vm_flags:00000070 anon_vma:
   (null) mapping:ffff88007c314058 index:141
[    1.790564] file:libdl-2.19.so fault:ext4_filemap_fault
mmap:ext4_file_mmap readpage:ext4_readpage

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1342339

FromLaura Abbott <labbott@redhat.com>
Date2016-02-24 20:00 +0100
Message-ID<r5MPf-cO-13@gated-at.bofh.it>
In reply to#1341142
On 02/23/2016 02:37 PM, Kees Cook wrote:
> On Tue, Feb 23, 2016 at 1:25 PM, Kees Cook <keescook@chromium.org> wrote:
>
>> zero poison only:
>> DEBUG_PAGEALLOC=n
>> PAGE_POISONING=y
>> PAGE_POISONING_NO_SANITY=y
>> PAGE_POISONING_ZERO=y
>> page_poison=on
>
> This combo (in next-20160223) results in an unusable system. :(
>
> [    1.754183] random: init urandom read with 11 bits of entropy available
> [    1.768449] hostname (1171) used greatest stack depth: 11808 bytes left
> [    1.787954] BUG: Bad page map in process init  pte:3d656c6f736e6f63
> pmd:00020067
> [    1.789177] addr:00007f9f68200000 vm_flags:00000070 anon_vma:
>     (null) mapping:ffff88007c314058 index:141
> [    1.790564] file:libdl-2.19.so fault:ext4_filemap_fault
> mmap:ext4_file_mmap readpage:ext4_readpage
>
> -Kees
>

Hmmm, it looks like the -mm tree currently only has the v1 of the
poisoning patches and not the v2. The v1 had a bug which would
cause issues like this due to some pages not getting zeroed fully. I should
follow up on that today.

Thanks,
Laura

[toc] | [prev] | [next] | [standalone]

#1342244

FromKees Cook <keescook@chromium.org>
Date2016-02-24 18:30 +0100
Message-ID<r5Lqa-7M2-15@gated-at.bofh.it>
In reply to#1341038
I did 3 defconfig builds as a benchmark, just to get ballpark numbers...

On Tue, Feb 23, 2016 at 1:25 PM, Kees Cook <keescook@chromium.org> wrote:
> Okay, it looks like the combinations to test are:
>
> default:
> DEBUG_PAGEALLOC=n
> PAGE_POISONING=n

Run times: 412.57 414.19 417.27
Mean: 414.68
Std Dev: 1.95

READ_AFTER_FREE fails:
[   83.521712] lkdtm: Performing direct entry READ_AFTER_FREE
[   83.521851] lkdtm: Value in memory before free: 12345678
[   83.521861] lkdtm: Attempting to read from freed memory
[   83.521864] lkdtm: Successfully read value: 12345678

WRITE_AFTER_FREE accidentally(?) gets detected (due to
CONFIG_SLUB_DEBUG=y default) but does not kill process:
[  120.544198] lkdtm: Performing direct entry WRITE_AFTER_FREE
[  120.544874] =============================================================================
[  120.545028] BUG kmalloc-1024 (Not tainted): Freepointer corrupt

READ_BUDDY_AFTER_FREE fails:
[  120.711466] lkdtm: Performing direct entry READ_BUDDY_AFTER_FREE
[  120.711472] lkdtm: Value in memory before free: 12345678
[  120.711473] lkdtm: Attempting to read from freed memory
[  120.711475] lkdtm: Successfully read value: 12345678

WRITE_BUDDY_AFTER_FREE fails:
[  120.714371] lkdtm: Performing direct entry WRITE_BUDDY_AFTER_FREE
[  120.714374] lkdtm: Writing to the buddy page before free
[  120.714377] lkdtm: Writing to the buddy page after free
[  120.714378] lkdtm: Wrote to free page successfully

So, other than what SLUB_DEBUG caught (which I think is probably an
accident, as we can change the test to avoid SLUB_DEBUG detection),
this is as-expected: use-after-free was not detected.

> heavy-duty:
> DEBUG_PAGEALLOC=y (ARCH_SUPPORTS_DEBUG_PAGEALLOC=y)
> debug_pagealloc=on

Run times: 486.82 464.90 469.34
Mean: 473.69
Std Dev: 9.46

Kind of a giant std-dev, but regardless, DEBUG_PAGEALLOC appears to
introduce a 14% perf overhead.

READ_AFTER_FREE fails, as before.
WRITE_AFTER_FREE accidentally gets detected, as before.

READ_BUDDY_AFTER_FREE detected:
[ 1760.522979] lkdtm: Performing direct entry READ_BUDDY_AFTER_FREE
[ 1760.522985] lkdtm: Value in memory before free: 12345678
[ 1760.523013] BUG: unable to handle kernel paging request at ffff88007994b000

WRITE_BUDDY_AFTER_FREE detected:
[ 1938.050091] lkdtm: Performing direct entry WRITE_BUDDY_AFTER_FREE
[ 1938.051475] lkdtm: Writing to the buddy page before free
[ 1938.056455] lkdtm: Writing to the buddy page after free
[ 1938.058543] BUG: unable to handle kernel paging request at ffff880079b38000

I was expecting DEBUG_PAGEALLOC to detect the read/write after free cases too.

> random poison only:
> DEBUG_PAGEALLOC=n
> PAGE_POISONING=y
> PAGE_POISONING_NO_SANITY=y
> PAGE_POISONING_ZERO=n
> page_poison=on

Run times: 428.96 424.76 426.12
Mean: 426.61
Std Dev: 1.75

This is under 3% perf overhead, with similar std-dev.

READ_AFTER_FREE fails, as before.
WRITE_AFTER_FREE accidentally gets detected, as before.

READ_BUDDY_AFTER_FREE detected:
[ 1448.167650] lkdtm: Performing direct entry READ_BUDDY_AFTER_FREE
[ 1448.167654] lkdtm: Value in memory before free: 12345678
[ 1448.167656] lkdtm: Attempting to read from freed memory
[ 1448.167677] general protection fault: 0000 [#1] SMP

WRITE_BUDDY_AFTER_FREE fails:
[ 1448.206587] lkdtm: Performing direct entry WRITE_BUDDY_AFTER_FREE
[ 1448.207550] lkdtm: Writing to the buddy page before free
[ 1448.208327] lkdtm: Writing to the buddy page after free
[ 1448.209088] lkdtm: Wrote to free page successfully

I wasn't expecting the GP with NO_SANITY=y. I was expecting to read
back random data?

> zero poison only:
> DEBUG_PAGEALLOC=n
> PAGE_POISONING=y
> PAGE_POISONING_NO_SANITY=y
> PAGE_POISONING_ZERO=y
> page_poison=on

This breaks my test system (as mentioned in other thread).

> random poison with sanity:
> DEBUG_PAGEALLOC=n
> PAGE_POISONING=y
> PAGE_POISONING_NO_SANITY=n
> PAGE_POISONING_ZERO=n
> page_poison=on

Run times: 432.72 434.41 429.19
Mean: 432.11
Std Dev: 2.18

Just over 4% perf overhead to no checks, and only 1.5% more overhead
compared to no sanity.

READ_AFTER_FREE fails, as before, but I thought it should be detected
with the sanity checking?
WRITE_AFTER_FREE accidentally gets detected, as before.

READ_BUDDY_AFTER_FREE detects:
[   46.720158] lkdtm: Performing direct entry READ_BUDDY_AFTER_FREE
[   46.720163] lkdtm: Value in memory before free: 12345678
[   46.720164] lkdtm: Attempting to read from freed memory
[   46.720169] general protection fault: 0000 [#1] SMP

WRITE_BUDDY_AFTER_FREE detects, but does not kill process:
[   46.756109] lkdtm: Performing direct entry WRITE_BUDDY_AFTER_FREE
[   46.756975] lkdtm: Writing to the buddy page before free
[   46.757764] lkdtm: Writing to the buddy page after free
[   46.758514] lkdtm: Wrote to free page successfully
[   46.759244] pagealloc: memory corruption

While WRITE_BUDDY_AFTER_FREE is detected, it doesn't kill the process
(similar to the WRITE_AFTER_FREE detection). We should change this
(maybe optionally) to drop processes that are triggering issues like
this.

> zero poison with sanity:
> DEBUG_PAGEALLOC=n
> PAGE_POISONING=y
> PAGE_POISONING_NO_SANITY=n
> PAGE_POISONING_ZERO=y
> page_poison=on

Again, couldn't test.

Looks like good progress to me. :)

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1342376

FromLaura Abbott <labbott@redhat.com>
Date2016-02-24 20:50 +0100
Message-ID<r5NBE-M1-5@gated-at.bofh.it>
In reply to#1342244
On 02/24/2016 09:22 AM, Kees Cook wrote:
> I did 3 defconfig builds as a benchmark, just to get ballpark numbers...
>
> On Tue, Feb 23, 2016 at 1:25 PM, Kees Cook <keescook@chromium.org> wrote:
>> Okay, it looks like the combinations to test are:
>>
>> default:
>> DEBUG_PAGEALLOC=n
>> PAGE_POISONING=n
>
> Run times: 412.57 414.19 417.27
> Mean: 414.68
> Std Dev: 1.95
>
> READ_AFTER_FREE fails:
> [   83.521712] lkdtm: Performing direct entry READ_AFTER_FREE
> [   83.521851] lkdtm: Value in memory before free: 12345678
> [   83.521861] lkdtm: Attempting to read from freed memory
> [   83.521864] lkdtm: Successfully read value: 12345678
>
> WRITE_AFTER_FREE accidentally(?) gets detected (due to
> CONFIG_SLUB_DEBUG=y default) but does not kill process:
> [  120.544198] lkdtm: Performing direct entry WRITE_AFTER_FREE
> [  120.544874] =============================================================================
> [  120.545028] BUG kmalloc-1024 (Not tainted): Freepointer corrupt
>
> READ_BUDDY_AFTER_FREE fails:
> [  120.711466] lkdtm: Performing direct entry READ_BUDDY_AFTER_FREE
> [  120.711472] lkdtm: Value in memory before free: 12345678
> [  120.711473] lkdtm: Attempting to read from freed memory
> [  120.711475] lkdtm: Successfully read value: 12345678
>
> WRITE_BUDDY_AFTER_FREE fails:
> [  120.714371] lkdtm: Performing direct entry WRITE_BUDDY_AFTER_FREE
> [  120.714374] lkdtm: Writing to the buddy page before free
> [  120.714377] lkdtm: Writing to the buddy page after free
> [  120.714378] lkdtm: Wrote to free page successfully
>
> So, other than what SLUB_DEBUG caught (which I think is probably an
> accident, as we can change the test to avoid SLUB_DEBUG detection),
> this is as-expected: use-after-free was not detected.
>
>> heavy-duty:
>> DEBUG_PAGEALLOC=y (ARCH_SUPPORTS_DEBUG_PAGEALLOC=y)
>> debug_pagealloc=on
>
> Run times: 486.82 464.90 469.34
> Mean: 473.69
> Std Dev: 9.46
>
> Kind of a giant std-dev, but regardless, DEBUG_PAGEALLOC appears to
> introduce a 14% perf overhead.
>
> READ_AFTER_FREE fails, as before.
> WRITE_AFTER_FREE accidentally gets detected, as before.
>
> READ_BUDDY_AFTER_FREE detected:
> [ 1760.522979] lkdtm: Performing direct entry READ_BUDDY_AFTER_FREE
> [ 1760.522985] lkdtm: Value in memory before free: 12345678
> [ 1760.523013] BUG: unable to handle kernel paging request at ffff88007994b000
>
> WRITE_BUDDY_AFTER_FREE detected:
> [ 1938.050091] lkdtm: Performing direct entry WRITE_BUDDY_AFTER_FREE
> [ 1938.051475] lkdtm: Writing to the buddy page before free
> [ 1938.056455] lkdtm: Writing to the buddy page after free
> [ 1938.058543] BUG: unable to handle kernel paging request at ffff880079b38000
>
> I was expecting DEBUG_PAGEALLOC to detect the read/write after free cases too.
>

Do you mean trigger a paging failure for the slab read/write free cases as well?
That generally won't occur for slab allocations. The slab allocator works by
allowing multiple allocations to exist on the same page. DEBUG_PAGEALLOC works on
a PAGE_SIZE granularity so as long as there are other allocation on the same slab
page it needs to be mapped. It may also be possible for a slab page to have no
allocated objects but still not be freed back into the buddy allocator.

Basically, DEBUG_PAGEALLOC will only kick in if a page is really and truly free
in the buddy allocator.
  
>> random poison only:
>> DEBUG_PAGEALLOC=n
>> PAGE_POISONING=y
>> PAGE_POISONING_NO_SANITY=y
>> PAGE_POISONING_ZERO=n
>> page_poison=on
>
> Run times: 428.96 424.76 426.12
> Mean: 426.61
> Std Dev: 1.75
>
> This is under 3% perf overhead, with similar std-dev.
>
> READ_AFTER_FREE fails, as before.
> WRITE_AFTER_FREE accidentally gets detected, as before.
>
> READ_BUDDY_AFTER_FREE detected:
> [ 1448.167650] lkdtm: Performing direct entry READ_BUDDY_AFTER_FREE
> [ 1448.167654] lkdtm: Value in memory before free: 12345678
> [ 1448.167656] lkdtm: Attempting to read from freed memory
> [ 1448.167677] general protection fault: 0000 [#1] SMP
>
> WRITE_BUDDY_AFTER_FREE fails:
> [ 1448.206587] lkdtm: Performing direct entry WRITE_BUDDY_AFTER_FREE
> [ 1448.207550] lkdtm: Writing to the buddy page before free
> [ 1448.208327] lkdtm: Writing to the buddy page after free
> [ 1448.209088] lkdtm: Wrote to free page successfully
>
> I wasn't expecting the GP with NO_SANITY=y. I was expecting to read
> back random data?
>

NO_SANITY just means that page poisoning isn't going to run the sanity
checks to check for bit flips or ovewritten poison. It doesn't have
any impact on how the poisoning works.
  
>> zero poison only:
>> DEBUG_PAGEALLOC=n
>> PAGE_POISONING=y
>> PAGE_POISONING_NO_SANITY=y
>> PAGE_POISONING_ZERO=y
>> page_poison=on
>
> This breaks my test system (as mentioned in other thread).
>

Yep, looks like the v1 patches and not the v2 patches which fix
a known issue with the zeroing.
  
>> random poison with sanity:
>> DEBUG_PAGEALLOC=n
>> PAGE_POISONING=y
>> PAGE_POISONING_NO_SANITY=n
>> PAGE_POISONING_ZERO=n
>> page_poison=on
>
> Run times: 432.72 434.41 429.19
> Mean: 432.11
> Std Dev: 2.18
>
> Just over 4% perf overhead to no checks, and only 1.5% more overhead
> compared to no sanity.
>
> READ_AFTER_FREE fails, as before, but I thought it should be detected
> with the sanity checking?
> WRITE_AFTER_FREE accidentally gets detected, as before.
>
> READ_BUDDY_AFTER_FREE detects:
> [   46.720158] lkdtm: Performing direct entry READ_BUDDY_AFTER_FREE
> [   46.720163] lkdtm: Value in memory before free: 12345678
> [   46.720164] lkdtm: Attempting to read from freed memory
> [   46.720169] general protection fault: 0000 [#1] SMP
>
> WRITE_BUDDY_AFTER_FREE detects, but does not kill process:
> [   46.756109] lkdtm: Performing direct entry WRITE_BUDDY_AFTER_FREE
> [   46.756975] lkdtm: Writing to the buddy page before free
> [   46.757764] lkdtm: Writing to the buddy page after free
> [   46.758514] lkdtm: Wrote to free page successfully
> [   46.759244] pagealloc: memory corruption
>
> While WRITE_BUDDY_AFTER_FREE is detected, it doesn't kill the process
> (similar to the WRITE_AFTER_FREE detection). We should change this
> (maybe optionally) to drop processes that are triggering issues like
> this.
>

So an optional call to panic when SL*B or page corruption is detected?
  
>> zero poison with sanity:
>> DEBUG_PAGEALLOC=n
>> PAGE_POISONING=y
>> PAGE_POISONING_NO_SANITY=n
>> PAGE_POISONING_ZERO=y
>> page_poison=on
>
> Again, couldn't test.
>
> Looks like good progress to me. :)
>

Thanks!

Laura

  
> -Kees
>

[toc] | [prev] | [next] | [standalone]

#1342460

FromKees Cook <keescook@chromium.org>
Date2016-02-24 22:50 +0100
Message-ID<r5PtM-2bk-11@gated-at.bofh.it>
In reply to#1342376
On Wed, Feb 24, 2016 at 11:40 AM, Laura Abbott <labbott@redhat.com> wrote:
> On 02/24/2016 09:22 AM, Kees Cook wrote:
>>
>> I did 3 defconfig builds as a benchmark, just to get ballpark numbers...
>>
>> On Tue, Feb 23, 2016 at 1:25 PM, Kees Cook <keescook@chromium.org> wrote:
>>>
>>> Okay, it looks like the combinations to test are:
>>>
>>> default:
>>> DEBUG_PAGEALLOC=n
>>> PAGE_POISONING=n
>>
>>
>> Run times: 412.57 414.19 417.27
>> Mean: 414.68
>> Std Dev: 1.95
>>
>> READ_AFTER_FREE fails:
>> [   83.521712] lkdtm: Performing direct entry READ_AFTER_FREE
>> [   83.521851] lkdtm: Value in memory before free: 12345678
>> [   83.521861] lkdtm: Attempting to read from freed memory
>> [   83.521864] lkdtm: Successfully read value: 12345678
>>
>> WRITE_AFTER_FREE accidentally(?) gets detected (due to
>> CONFIG_SLUB_DEBUG=y default) but does not kill process:
>> [  120.544198] lkdtm: Performing direct entry WRITE_AFTER_FREE
>> [  120.544874]
>> =============================================================================
>> [  120.545028] BUG kmalloc-1024 (Not tainted): Freepointer corrupt

Okay, I've fixed this test to write into the middle, similar to the
READ_AFTER_FREE test, now it no longer detects the write.

>> READ_BUDDY_AFTER_FREE fails:
>> [  120.711466] lkdtm: Performing direct entry READ_BUDDY_AFTER_FREE
>> [  120.711472] lkdtm: Value in memory before free: 12345678
>> [  120.711473] lkdtm: Attempting to read from freed memory
>> [  120.711475] lkdtm: Successfully read value: 12345678
>>
>> WRITE_BUDDY_AFTER_FREE fails:
>> [  120.714371] lkdtm: Performing direct entry WRITE_BUDDY_AFTER_FREE
>> [  120.714374] lkdtm: Writing to the buddy page before free
>> [  120.714377] lkdtm: Writing to the buddy page after free
>> [  120.714378] lkdtm: Wrote to free page successfully
>>
>> So, other than what SLUB_DEBUG caught (which I think is probably an
>> accident, as we can change the test to avoid SLUB_DEBUG detection),
>> this is as-expected: use-after-free was not detected.
>>
>>> heavy-duty:
>>> DEBUG_PAGEALLOC=y (ARCH_SUPPORTS_DEBUG_PAGEALLOC=y)
>>> debug_pagealloc=on
>>
>>
>> Run times: 486.82 464.90 469.34
>> Mean: 473.69
>> Std Dev: 9.46
>>
>> Kind of a giant std-dev, but regardless, DEBUG_PAGEALLOC appears to
>> introduce a 14% perf overhead.
>>
>> READ_AFTER_FREE fails, as before.
>> WRITE_AFTER_FREE accidentally gets detected, as before.
>>
>> READ_BUDDY_AFTER_FREE detected:
>> [ 1760.522979] lkdtm: Performing direct entry READ_BUDDY_AFTER_FREE
>> [ 1760.522985] lkdtm: Value in memory before free: 12345678
>> [ 1760.523013] BUG: unable to handle kernel paging request at
>> ffff88007994b000
>>
>> WRITE_BUDDY_AFTER_FREE detected:
>> [ 1938.050091] lkdtm: Performing direct entry WRITE_BUDDY_AFTER_FREE
>> [ 1938.051475] lkdtm: Writing to the buddy page before free
>> [ 1938.056455] lkdtm: Writing to the buddy page after free
>> [ 1938.058543] BUG: unable to handle kernel paging request at
>> ffff880079b38000
>>
>> I was expecting DEBUG_PAGEALLOC to detect the read/write after free cases
>> too.
>>
>
> Do you mean trigger a paging failure for the slab read/write free cases as
> well?
> That generally won't occur for slab allocations. The slab allocator works by
> allowing multiple allocations to exist on the same page. DEBUG_PAGEALLOC
> works on
> a PAGE_SIZE granularity so as long as there are other allocation on the same
> slab
> page it needs to be mapped. It may also be possible for a slab page to have
> no
> allocated objects but still not be freed back into the buddy allocator.
>
> Basically, DEBUG_PAGEALLOC will only kick in if a page is really and truly
> free
> in the buddy allocator.

Okay, I think I understand. Seems like the buddy test is finer
granularity, essentially.

>>> random poison only:
>>> DEBUG_PAGEALLOC=n
>>> PAGE_POISONING=y
>>> PAGE_POISONING_NO_SANITY=y
>>> PAGE_POISONING_ZERO=n
>>> page_poison=on
>>
>>
>> Run times: 428.96 424.76 426.12
>> Mean: 426.61
>> Std Dev: 1.75
>>
>> This is under 3% perf overhead, with similar std-dev.
>>
>> READ_AFTER_FREE fails, as before.
>> WRITE_AFTER_FREE accidentally gets detected, as before.
>>
>> READ_BUDDY_AFTER_FREE detected:
>> [ 1448.167650] lkdtm: Performing direct entry READ_BUDDY_AFTER_FREE
>> [ 1448.167654] lkdtm: Value in memory before free: 12345678
>> [ 1448.167656] lkdtm: Attempting to read from freed memory
>> [ 1448.167677] general protection fault: 0000 [#1] SMP
>>
>> WRITE_BUDDY_AFTER_FREE fails:
>> [ 1448.206587] lkdtm: Performing direct entry WRITE_BUDDY_AFTER_FREE
>> [ 1448.207550] lkdtm: Writing to the buddy page before free
>> [ 1448.208327] lkdtm: Writing to the buddy page after free
>> [ 1448.209088] lkdtm: Wrote to free page successfully
>>
>> I wasn't expecting the GP with NO_SANITY=y. I was expecting to read
>> back random data?
>>
>
> NO_SANITY just means that page poisoning isn't going to run the sanity
> checks to check for bit flips or ovewritten poison. It doesn't have
> any impact on how the poisoning works.

Okay, I think I understand. Another thing, though: I wasn't expecting
READ_AFTER_FREE to return unpoisoned data.

>
>>>
>>> zero poison only:
>>> DEBUG_PAGEALLOC=n
>>> PAGE_POISONING=y
>>> PAGE_POISONING_NO_SANITY=y
>>> PAGE_POISONING_ZERO=y
>>> page_poison=on
>>
>>
>> This breaks my test system (as mentioned in other thread).
>>
>
> Yep, looks like the v1 patches and not the v2 patches which fix
> a known issue with the zeroing.

Ah-ha, I'll go find those and retest.

-Kees

>>> random poison with sanity:
>>> DEBUG_PAGEALLOC=n
>>> PAGE_POISONING=y
>>> PAGE_POISONING_NO_SANITY=n
>>> PAGE_POISONING_ZERO=n
>>> page_poison=on
>>
>>
>> Run times: 432.72 434.41 429.19
>> Mean: 432.11
>> Std Dev: 2.18
>>
>> Just over 4% perf overhead to no checks, and only 1.5% more overhead
>> compared to no sanity.
>>
>> READ_AFTER_FREE fails, as before, but I thought it should be detected
>> with the sanity checking?
>> WRITE_AFTER_FREE accidentally gets detected, as before.
>>
>> READ_BUDDY_AFTER_FREE detects:
>> [   46.720158] lkdtm: Performing direct entry READ_BUDDY_AFTER_FREE
>> [   46.720163] lkdtm: Value in memory before free: 12345678
>> [   46.720164] lkdtm: Attempting to read from freed memory
>> [   46.720169] general protection fault: 0000 [#1] SMP
>>
>> WRITE_BUDDY_AFTER_FREE detects, but does not kill process:
>> [   46.756109] lkdtm: Performing direct entry WRITE_BUDDY_AFTER_FREE
>> [   46.756975] lkdtm: Writing to the buddy page before free
>> [   46.757764] lkdtm: Writing to the buddy page after free
>> [   46.758514] lkdtm: Wrote to free page successfully
>> [   46.759244] pagealloc: memory corruption
>>
>> While WRITE_BUDDY_AFTER_FREE is detected, it doesn't kill the process
>> (similar to the WRITE_AFTER_FREE detection). We should change this
>> (maybe optionally) to drop processes that are triggering issues like
>> this.
>>
>
> So an optional call to panic when SL*B or page corruption is detected?
>
>>>
>>> zero poison with sanity:
>>> DEBUG_PAGEALLOC=n
>>> PAGE_POISONING=y
>>> PAGE_POISONING_NO_SANITY=n
>>> PAGE_POISONING_ZERO=y
>>> page_poison=on
>>
>>
>> Again, couldn't test.
>>
>> Looks like good progress to me. :)
>>
>
> Thanks!
>
> Laura
>
>
>>
>> -Kees
>>
>



-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1342523

FromKees Cook <keescook@chromium.org>
Date2016-02-25 00:40 +0100
Message-ID<r5Rce-3uS-17@gated-at.bofh.it>
In reply to#1342460
On Wed, Feb 24, 2016 at 1:48 PM, Kees Cook <keescook@chromium.org> wrote:
> On Wed, Feb 24, 2016 at 11:40 AM, Laura Abbott <labbott@redhat.com> wrote:
>> Yep, looks like the v1 patches and not the v2 patches which fix
>> a known issue with the zeroing.
>
> Ah-ha, I'll go find those and retest.

I sent out a series that was rebased. It works for me, but I want to
make sure I didn't make any glaring issues. I've also sent some fixes
to the lkdtm tests. One thing that stands out to me still is that the
READ_AFTER_FREE never shows poisoning. I remain confused, since
obviously if zeroing is working, it's being correctly poisoned...

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1342569

FromLaura Abbott <labbott@redhat.com>
Date2016-02-25 02:30 +0100
Message-ID<r5SUG-4Ka-11@gated-at.bofh.it>
In reply to#1342523
On 02/24/2016 03:37 PM, Kees Cook wrote:
> On Wed, Feb 24, 2016 at 1:48 PM, Kees Cook <keescook@chromium.org> wrote:
>> On Wed, Feb 24, 2016 at 11:40 AM, Laura Abbott <labbott@redhat.com> wrote:
>>> Yep, looks like the v1 patches and not the v2 patches which fix
>>> a known issue with the zeroing.
>>
>> Ah-ha, I'll go find those and retest.
>
> I sent out a series that was rebased. It works for me, but I want to
> make sure I didn't make any glaring issues. I've also sent some fixes
> to the lkdtm tests. One thing that stands out to me still is that the
> READ_AFTER_FREE never shows poisoning. I remain confused, since
> obviously if zeroing is working, it's being correctly poisoned...
>
> -Kees
>

I'll review the rebased series you sent out for the page poisoning patches.
If it's okay with you, I'll pull in the updates to the LKDTM test. If you
test with slub_debug=P on the command line do you see the READ_AFTER_FREE
test working as expected? Setting that on the command line will set up
the poisoning which should make the READ_AFTER_FREE test fail.

Thanks,
Laura

[toc] | [prev] | [next] | [standalone]

#1343352

FromKees Cook <keescook@chromium.org>
Date2016-02-25 18:40 +0100
Message-ID<r683p-78A-25@gated-at.bofh.it>
In reply to#1342569
On Wed, Feb 24, 2016 at 5:28 PM, Laura Abbott <labbott@redhat.com> wrote:
> On 02/24/2016 03:37 PM, Kees Cook wrote:
>>
>> On Wed, Feb 24, 2016 at 1:48 PM, Kees Cook <keescook@chromium.org> wrote:
>>>
>>> On Wed, Feb 24, 2016 at 11:40 AM, Laura Abbott <labbott@redhat.com>
>>> wrote:
>>>>
>>>> Yep, looks like the v1 patches and not the v2 patches which fix
>>>> a known issue with the zeroing.
>>>
>>>
>>> Ah-ha, I'll go find those and retest.
>>
>>
>> I sent out a series that was rebased. It works for me, but I want to
>> make sure I didn't make any glaring issues. I've also sent some fixes
>> to the lkdtm tests. One thing that stands out to me still is that the
>> READ_AFTER_FREE never shows poisoning. I remain confused, since
>> obviously if zeroing is working, it's being correctly poisoned...
>>
>> -Kees
>>
>
> I'll review the rebased series you sent out for the page poisoning patches.
> If it's okay with you, I'll pull in the updates to the LKDTM test.

Yes, please feel free!

> If you
> test with slub_debug=P on the command line do you see the READ_AFTER_FREE
> test working as expected? Setting that on the command line will set up
> the poisoning which should make the READ_AFTER_FREE test fail.

Ah-ha, yes, that was one of the missing pieces:

[   10.790970] lkdtm: Performing direct entry READ_AFTER_FREE
[   10.790992] lkdtm: Value in memory before free: 12345678
[   10.790996] lkdtm: Attempting bad read from freed memory
[   10.790998] lkdtm: Memory correctly poisoned, calling BUG
[   10.791067] ------------[ cut here ]------------
[   10.792037] kernel BUG at drivers/misc/lkdtm.c:465!

I see that "F" is also needed to do the sanity checks, but the poison
ends up being different again from what I was expected:

[    8.643902] lkdtm: Performing direct entry WRITE_AFTER_FREE
[    8.645215] lkdtm: Allocated memory ffff88007b446850-ffff88007b446c50
[    8.646700] lkdtm: Attempting bad write to freed memory at ffff88007b446a50
[    8.648295] =============================================================================
[    8.649275] BUG kmalloc-1024 (Tainted: G      D        ): Poison overwritten
[    8.649275] -----------------------------------------------------------------------------
[    8.649275]
[    8.649275] INFO: 0xffff88007b446a50-0xffff88007b446a53. First byte
0xf0 instead of 0x6b

0x6b is POISON_FREE:

#define POISON_INUSE    0x5a    /* for use-uninitialised poisoning */
#define POISON_FREE     0x6b    /* for use-after-free poisoning */
#define POISON_END      0xa5    /* end-byte of poisoning */

So it seems like there are separate poisonings going on? Modifying
READ_AFTER_FREE a bit more, I see that it looks like only the buddy
allocator is getting the zero poisoning?

[   61.755450] lkdtm: Performing direct entry READ_AFTER_FREE
[   61.757436] lkdtm: Value in memory before free: 12345678
[   61.759390] lkdtm: Attempting bad read from freed memory
[   61.761649] lkdtm: Memory correctly poisoned (6b6b6b6b)

[   62.139408] lkdtm: Performing direct entry READ_BUDDY_AFTER_FREE
[   62.140766] lkdtm: Value in memory before free: 12345678
[   62.141989] lkdtm: Attempting to read from freed memory
[   62.143225] lkdtm: Memory correctly poisoned (0)

Once this series is in, we need to find a way to make a single CONFIG
to be more friendly than needing to add "page_poison=on slub_debug=FP"
to the command line. :)

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1343574

FromLaura Abbott <labbott@redhat.com>
Date2016-02-26 00:20 +0100
Message-ID<r6dmq-2FR-29@gated-at.bofh.it>
In reply to#1343352
On 02/25/2016 09:35 AM, Kees Cook wrote:
> On Wed, Feb 24, 2016 at 5:28 PM, Laura Abbott <labbott@redhat.com> wrote:
>> On 02/24/2016 03:37 PM, Kees Cook wrote:
>>>
>>> On Wed, Feb 24, 2016 at 1:48 PM, Kees Cook <keescook@chromium.org> wrote:
>>>>
>>>> On Wed, Feb 24, 2016 at 11:40 AM, Laura Abbott <labbott@redhat.com>
>>>> wrote:
>>>>>
>>>>> Yep, looks like the v1 patches and not the v2 patches which fix
>>>>> a known issue with the zeroing.
>>>>
>>>>
>>>> Ah-ha, I'll go find those and retest.
>>>
>>>
>>> I sent out a series that was rebased. It works for me, but I want to
>>> make sure I didn't make any glaring issues. I've also sent some fixes
>>> to the lkdtm tests. One thing that stands out to me still is that the
>>> READ_AFTER_FREE never shows poisoning. I remain confused, since
>>> obviously if zeroing is working, it's being correctly poisoned...
>>>
>>> -Kees
>>>
>>
>> I'll review the rebased series you sent out for the page poisoning patches.
>> If it's okay with you, I'll pull in the updates to the LKDTM test.
>
> Yes, please feel free!
>
>> If you
>> test with slub_debug=P on the command line do you see the READ_AFTER_FREE
>> test working as expected? Setting that on the command line will set up
>> the poisoning which should make the READ_AFTER_FREE test fail.
>
> Ah-ha, yes, that was one of the missing pieces:
>
> [   10.790970] lkdtm: Performing direct entry READ_AFTER_FREE
> [   10.790992] lkdtm: Value in memory before free: 12345678
> [   10.790996] lkdtm: Attempting bad read from freed memory
> [   10.790998] lkdtm: Memory correctly poisoned, calling BUG
> [   10.791067] ------------[ cut here ]------------
> [   10.792037] kernel BUG at drivers/misc/lkdtm.c:465!
>
> I see that "F" is also needed to do the sanity checks, but the poison
> ends up being different again from what I was expected:
>
> [    8.643902] lkdtm: Performing direct entry WRITE_AFTER_FREE
> [    8.645215] lkdtm: Allocated memory ffff88007b446850-ffff88007b446c50
> [    8.646700] lkdtm: Attempting bad write to freed memory at ffff88007b446a50
> [    8.648295] =============================================================================
> [    8.649275] BUG kmalloc-1024 (Tainted: G      D        ): Poison overwritten
> [    8.649275] -----------------------------------------------------------------------------
> [    8.649275]
> [    8.649275] INFO: 0xffff88007b446a50-0xffff88007b446a53. First byte
> 0xf0 instead of 0x6b
>
> 0x6b is POISON_FREE:
>
> #define POISON_INUSE    0x5a    /* for use-uninitialised poisoning */
> #define POISON_FREE     0x6b    /* for use-after-free poisoning */
> #define POISON_END      0xa5    /* end-byte of poisoning */
>

Yep, 0x6b is a magic number I've seen all too frequently before ;)

The current poisoning with slub_debug=P covers multiple cases. On
alloc, the memory is set with POISON_INUSE to catch uninitailized
usage. on free, the memory is set to POISON_FREE To catch use after
free bugs. The last bit POISON_END is set at the end of the block
to catch users who might run off the end of the buffer. Having the
different values makes it easier to determine which bug it is.
  
> So it seems like there are separate poisonings going on? Modifying
> READ_AFTER_FREE a bit more, I see that it looks like only the buddy
> allocator is getting the zero poisoning?
>

Yes. The buddy allocator and SL*B allocators are two separate pieces
of code which need independent poisoning mechanisms. Currently, only
the buddy allocator has the zero poisoning. The same functionality
can be added to SL*B allocator as well if it seems beneficial.
  
> [   61.755450] lkdtm: Performing direct entry READ_AFTER_FREE
> [   61.757436] lkdtm: Value in memory before free: 12345678
> [   61.759390] lkdtm: Attempting bad read from freed memory
> [   61.761649] lkdtm: Memory correctly poisoned (6b6b6b6b)
>
> [   62.139408] lkdtm: Performing direct entry READ_BUDDY_AFTER_FREE
> [   62.140766] lkdtm: Value in memory before free: 12345678
> [   62.141989] lkdtm: Attempting to read from freed memory
> [   62.143225] lkdtm: Memory correctly poisoned (0)
>
> Once this series is in, we need to find a way to make a single CONFIG
> to be more friendly than needing to add "page_poison=on slub_debug=FP"
> to the command line. :)

Yep. We can probably use CONFIG_SLUB_DEBUG_ON as an example of what to
do.

On a side note, what's your opinion on the necessity of 'F' for the
checks? 'P' by itself will ensure the memory is cleared. The sanity
checks had a notable imapct on performance.

>
> -Kees
>

Thanks,
Laura

[toc] | [prev] | [next] | [standalone]

#1344392

FromKees Cook <keescook@chromium.org>
Date2016-02-26 17:10 +0100
Message-ID<r6t7S-5QF-57@gated-at.bofh.it>
In reply to#1343574
On Thu, Feb 25, 2016 at 3:15 PM, Laura Abbott <labbott@redhat.com> wrote:
> On 02/25/2016 09:35 AM, Kees Cook wrote:
>> Ah-ha, yes, that was one of the missing pieces:
>>
>> [   10.790970] lkdtm: Performing direct entry READ_AFTER_FREE
>> [   10.790992] lkdtm: Value in memory before free: 12345678
>> [   10.790996] lkdtm: Attempting bad read from freed memory
>> [   10.790998] lkdtm: Memory correctly poisoned, calling BUG
>> [   10.791067] ------------[ cut here ]------------
>> [   10.792037] kernel BUG at drivers/misc/lkdtm.c:465!
>>
>> I see that "F" is also needed to do the sanity checks, but the poison
>> ends up being different again from what I was expected:
>>
>> [    8.643902] lkdtm: Performing direct entry WRITE_AFTER_FREE
>> [    8.645215] lkdtm: Allocated memory ffff88007b446850-ffff88007b446c50
>> [    8.646700] lkdtm: Attempting bad write to freed memory at
>> ffff88007b446a50
>> [    8.648295]
>> =============================================================================
>> [    8.649275] BUG kmalloc-1024 (Tainted: G      D        ): Poison
>> overwritten
>> [    8.649275]
>> -----------------------------------------------------------------------------
>> [    8.649275]
>> [    8.649275] INFO: 0xffff88007b446a50-0xffff88007b446a53. First byte
>> 0xf0 instead of 0x6b
>>
>> 0x6b is POISON_FREE:
>>
>> #define POISON_INUSE    0x5a    /* for use-uninitialised poisoning */
>> #define POISON_FREE     0x6b    /* for use-after-free poisoning */
>> #define POISON_END      0xa5    /* end-byte of poisoning */
>>
>
> Yep, 0x6b is a magic number I've seen all too frequently before ;)
>
> The current poisoning with slub_debug=P covers multiple cases. On
> alloc, the memory is set with POISON_INUSE to catch uninitailized
> usage. on free, the memory is set to POISON_FREE To catch use after
> free bugs. The last bit POISON_END is set at the end of the block
> to catch users who might run off the end of the buffer. Having the
> different values makes it easier to determine which bug it is.
>
>>
>> So it seems like there are separate poisonings going on? Modifying
>> READ_AFTER_FREE a bit more, I see that it looks like only the buddy
>> allocator is getting the zero poisoning?
>>
>
> Yes. The buddy allocator and SL*B allocators are two separate pieces
> of code which need independent poisoning mechanisms. Currently, only
> the buddy allocator has the zero poisoning. The same functionality
> can be added to SL*B allocator as well if it seems beneficial.

My concerns are with the performance characteristics, mostly. To match
PAX_MEMORY_SANITIZE, zero poisoning almost everything should get us
into the 3% range, I'm hoping.

>> [   61.755450] lkdtm: Performing direct entry READ_AFTER_FREE
>> [   61.757436] lkdtm: Value in memory before free: 12345678
>> [   61.759390] lkdtm: Attempting bad read from freed memory
>> [   61.761649] lkdtm: Memory correctly poisoned (6b6b6b6b)
>>
>> [   62.139408] lkdtm: Performing direct entry READ_BUDDY_AFTER_FREE
>> [   62.140766] lkdtm: Value in memory before free: 12345678
>> [   62.141989] lkdtm: Attempting to read from freed memory
>> [   62.143225] lkdtm: Memory correctly poisoned (0)
>>
>> Once this series is in, we need to find a way to make a single CONFIG
>> to be more friendly than needing to add "page_poison=on slub_debug=FP"
>> to the command line. :)
>
> Yep. We can probably use CONFIG_SLUB_DEBUG_ON as an example of what to
> do.
>
> On a side note, what's your opinion on the necessity of 'F' for the
> checks? 'P' by itself will ensure the memory is cleared. The sanity
> checks had a notable imapct on performance.

Ah, no, that's just for completeness of testing. The sanity checks
appear to add about 3% additional overhead, but poisoning seems to add
about 9%. :(

DEBUG_PAGEALLOC=n
PAGE_POISONING=y
PAGE_POISONING_NO_SANITY=y
PAGE_POISONING_ZERO=y

Run times: 389.23 384.88 386.33
Mean: 386.81
Std Dev: 1.81

LKDTM detects nothing, as expected.


DEBUG_PAGEALLOC=n
PAGE_POISONING=y
PAGE_POISONING_NO_SANITY=y
PAGE_POISONING_ZERO=n
slub_debug=P page_poison=on

Run times: 435.63 419.20 422.82
Mean: 425.89
Std Dev: 7.05

Overhead: 9.2% vs all disabled

Poisoning confirmed: READ_AFTER_FREE, READ_BUDDY_AFTER_FREE
Writes not detected, as expected.


DEBUG_PAGEALLOC=n
PAGE_POISONING=y
PAGE_POISONING_NO_SANITY=y
PAGE_POISONING_ZERO=y
slub_debug=P page_poison=on

Run times: 423.44 422.32 424.95
Mean: 423.57
Std Dev: 1.08

Overhead 8.7% overhead vs disabled, 0.5% improvement over non-zero
poison (though only the buddy allocator is using the zero poison).

Poisoning confirmed: READ_AFTER_FREE, READ_BUDDY_AFTER_FREE
Writes not detected, as expected.


DEBUG_PAGEALLOC=n
PAGE_POISONING=y
PAGE_POISONING_NO_SANITY=n
PAGE_POISONING_ZERO=y
slub_debug=FP page_poison=on

Run times: 454.26 429.46 430.48
Mean: 438.07
Std Dev: 11.46

Overhead: 11.7% vs nothing, 3% more overhead than no sanitizing.

All four tests detect correctly.


-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1344745

FromLaura Abbott <labbott@redhat.com>
Date2016-02-26 23:20 +0100
Message-ID<r6yTU-1pV-21@gated-at.bofh.it>
In reply to#1344392
On 02/26/2016 08:03 AM, Kees Cook wrote:
> On Thu, Feb 25, 2016 at 3:15 PM, Laura Abbott <labbott@redhat.com> wrote:
>> On 02/25/2016 09:35 AM, Kees Cook wrote:
>>> Ah-ha, yes, that was one of the missing pieces:
>>>
>>> [   10.790970] lkdtm: Performing direct entry READ_AFTER_FREE
>>> [   10.790992] lkdtm: Value in memory before free: 12345678
>>> [   10.790996] lkdtm: Attempting bad read from freed memory
>>> [   10.790998] lkdtm: Memory correctly poisoned, calling BUG
>>> [   10.791067] ------------[ cut here ]------------
>>> [   10.792037] kernel BUG at drivers/misc/lkdtm.c:465!
>>>
>>> I see that "F" is also needed to do the sanity checks, but the poison
>>> ends up being different again from what I was expected:
>>>
>>> [    8.643902] lkdtm: Performing direct entry WRITE_AFTER_FREE
>>> [    8.645215] lkdtm: Allocated memory ffff88007b446850-ffff88007b446c50
>>> [    8.646700] lkdtm: Attempting bad write to freed memory at
>>> ffff88007b446a50
>>> [    8.648295]
>>> =============================================================================
>>> [    8.649275] BUG kmalloc-1024 (Tainted: G      D        ): Poison
>>> overwritten
>>> [    8.649275]
>>> -----------------------------------------------------------------------------
>>> [    8.649275]
>>> [    8.649275] INFO: 0xffff88007b446a50-0xffff88007b446a53. First byte
>>> 0xf0 instead of 0x6b
>>>
>>> 0x6b is POISON_FREE:
>>>
>>> #define POISON_INUSE    0x5a    /* for use-uninitialised poisoning */
>>> #define POISON_FREE     0x6b    /* for use-after-free poisoning */
>>> #define POISON_END      0xa5    /* end-byte of poisoning */
>>>
>>
>> Yep, 0x6b is a magic number I've seen all too frequently before ;)
>>
>> The current poisoning with slub_debug=P covers multiple cases. On
>> alloc, the memory is set with POISON_INUSE to catch uninitailized
>> usage. on free, the memory is set to POISON_FREE To catch use after
>> free bugs. The last bit POISON_END is set at the end of the block
>> to catch users who might run off the end of the buffer. Having the
>> different values makes it easier to determine which bug it is.
>>
>>>
>>> So it seems like there are separate poisonings going on? Modifying
>>> READ_AFTER_FREE a bit more, I see that it looks like only the buddy
>>> allocator is getting the zero poisoning?
>>>
>>
>> Yes. The buddy allocator and SL*B allocators are two separate pieces
>> of code which need independent poisoning mechanisms. Currently, only
>> the buddy allocator has the zero poisoning. The same functionality
>> can be added to SL*B allocator as well if it seems beneficial.
>
> My concerns are with the performance characteristics, mostly. To match
> PAX_MEMORY_SANITIZE, zero poisoning almost everything should get us
> into the 3% range, I'm hoping.
>

I did a quick hack of zero poisoning for the slab allocator and I
didn't see any improvement in hackbench performance which is fairly
sensitive to slab performance. This doesn't surprise me when I
actually think about it.

Before I sent out my last set of performance optimizations for
SLUB debug path, I did a profile with ftrace to see if there was
anything else quick I could do. My profiling showed that the
poisoning itself was not where most of the allocation time was
spent. 25-50% of the time was being spent in removing the CPU slab.
Considering poisoning means that the CPU slab is never really used,
this can probably be improved. It's worth noting that the
PAX_MEMORY_SANITIZE implementation still uses the fast path so it
isn't affected here. (The trade off is a minor penalty on the
fast path even when poisoning is disabled which isn't acceptable
to the maintainers currently.)

Basically, until we've optimized other things I don't think the
zero poisoning will have a significant effect on performance.
The next set of optimizations will involve changing some of the
guts of the SLUB allocator. I have some ideas how to approach this
but we'll see if they pan out.

Thanks,
Laura

[toc] | [prev] | [next] | [standalone]

#1344792

FromKees Cook <keescook@chromium.org>
Date2016-02-26 23:40 +0100
Message-ID<r6zdg-1zD-17@gated-at.bofh.it>
In reply to#1344745
On Fri, Feb 26, 2016 at 2:19 PM, Laura Abbott <labbott@redhat.com> wrote:
> I did a quick hack of zero poisoning for the slab allocator and I
> didn't see any improvement in hackbench performance which is fairly
> sensitive to slab performance. This doesn't surprise me when I
> actually think about it.
>
> Before I sent out my last set of performance optimizations for
> SLUB debug path, I did a profile with ftrace to see if there was
> anything else quick I could do. My profiling showed that the
> poisoning itself was not where most of the allocation time was
> spent. 25-50% of the time was being spent in removing the CPU slab.
> Considering poisoning means that the CPU slab is never really used,
> this can probably be improved. It's worth noting that the
> PAX_MEMORY_SANITIZE implementation still uses the fast path so it
> isn't affected here. (The trade off is a minor penalty on the
> fast path even when poisoning is disabled which isn't acceptable
> to the maintainers currently.)

Oh right, all of this is still the slow path...

> Basically, until we've optimized other things I don't think the
> zero poisoning will have a significant effect on performance.
> The next set of optimizations will involve changing some of the
> guts of the SLUB allocator. I have some ideas how to approach this
> but we'll see if they pan out.

And we can't just have a CONFIG for the fast-path sanitization? Then
it's not in anyone's way, etc?

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1346283

FromLaura Abbott <labbott@redhat.com>
Date2016-03-01 02:40 +0100
Message-ID<r7Hs6-2lK-13@gated-at.bofh.it>
In reply to#1344792
On 02/26/2016 02:33 PM, Kees Cook wrote:
> On Fri, Feb 26, 2016 at 2:19 PM, Laura Abbott <labbott@redhat.com> wrote:
>> I did a quick hack of zero poisoning for the slab allocator and I
>> didn't see any improvement in hackbench performance which is fairly
>> sensitive to slab performance. This doesn't surprise me when I
>> actually think about it.
>>
>> Before I sent out my last set of performance optimizations for
>> SLUB debug path, I did a profile with ftrace to see if there was
>> anything else quick I could do. My profiling showed that the
>> poisoning itself was not where most of the allocation time was
>> spent. 25-50% of the time was being spent in removing the CPU slab.
>> Considering poisoning means that the CPU slab is never really used,
>> this can probably be improved. It's worth noting that the
>> PAX_MEMORY_SANITIZE implementation still uses the fast path so it
>> isn't affected here. (The trade off is a minor penalty on the
>> fast path even when poisoning is disabled which isn't acceptable
>> to the maintainers currently.)
>
> Oh right, all of this is still the slow path...
>
>> Basically, until we've optimized other things I don't think the
>> zero poisoning will have a significant effect on performance.
>> The next set of optimizations will involve changing some of the
>> guts of the SLUB allocator. I have some ideas how to approach this
>> but we'll see if they pan out.
>
> And we can't just have a CONFIG for the fast-path sanitization? Then
> it's not in anyone's way, etc?
>

I proposed that but it was shot down :(

The request was to try and make the slow path work so I'm going to
do my due diligence there. If it turns out that it still isn't fast
enough I'll bring it up again but unless we can say that slow path
is still x% slower even after optimization I don't think it will fly.

> -Kees
>

Thanks,
Laura

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web