On 04/06/2018 03:14 PM, Andrey Konovalov wrote:
> On Thu, Apr 5, 2018 at 3:02 PM, Andrey Ryabinin <aryabi...@virtuozzo.com> 
> wrote:
>> On 04/04/2018 08:00 PM, Andrey Konovalov wrote:
>>> On Wed, Apr 4, 2018 at 2:39 PM, Andrey Ryabinin <aryabi...@virtuozzo.com> 
>>> wrote:
>>>>>> You can save tag somewhere in page struct and make page_address() return 
>>>>>> tagged address.
>>>>>> I'm not sure it might be even possible to squeeze the tag into 
>>>>>> page->flags on some configurations,
>>>>>> see include/linux/page-flags-layout.h
>>>>> One page can contain multiple objects with different tags, so we would
>>>>> need to save the tag for each of them.
>>>> What do you mean? Slab page? The per-page tag is needed only for !PageSlab 
>>>> pages.
>>>> For slab pages we have kmalloc/kmem_cache_alloc() which already return 
>>>> properly tagged address.
>>>> But the page allocator returns a pointer to struct page. One has to call 
>>>> page_address(page)
>>>> to use that page. Returning 'ignore-me'-tagged address from page_address() 
>>>> makes the whole
>>>> class of bugs invisible to KHWASAN. This is a serious downside comparing 
>>>> to classic KASAN which can
>>>> detect missuses of page allocator API.
>>> Yes, slab page. Here's an example:
>>> 1. do_get_write_access() allocates frozen_buffer with jbd2_alloc,
>>> which calls kmem_cache_alloc, and then saves the result to
>>> jh->b_frozen_data.
>>> 2. jbd2_journal_write_metadata_buffer() takes the value of
>>> jh_in->b_frozen_data and calls virt_to_page() (and offset_in_page())
>>> on it.
>>> 3. jbd2_journal_write_metadata_buffer() then calls kmap_atomic(),
>>> which calls page_address(), on the resulting page address.
>>> The tag gets erased. The page belongs to slab and can contain multiple
>>> objects with different tags.
>> I see. Ideally that kind of problem should be fixed by reworking/redesigning 
>> such code,
>> however jbd2_journal_write_metadata_buffer() is far from the only place which
>> does that trick. Fixing all of them would be a huge task probably, so 
>> ignoring such
>> accesses seems to be the only choice we have.
>> Nevertheless, this doesn't mean that we should ignore *all* accesses to 
>> !slab memory.
> So you mean we need to find a way to ignore accesses via pointers
> returned by page_address(), but still check accesses through all other
> pointers tagged with 0xFF? I don't see an obvious way to do this. I'm
> open to suggestions though.

I'm saying that we need to ignore accesses to slab objects if pointer
to slab object obtained via page_address() + offset_in_page() trick, but don't 
anything else.

So, save tag somewhere in page struct and poison shadow with that tag. Make 
page_address() to
return tagged address for all !PageSlab() pages. For PageSlab() pages 
page_address() should return
0xff tagged address, so we could ignore such accesses.

kvmarm mailing list

Reply via email to