Re: [RFC PATCH v2 13/15] khwasan: add hooks implementation

[Andrey Ryabinin <aryabinin@virtuozzo.com>]

On 04/12/2018 07:45 PM, Andrey Konovalov wrote:
> On Tue, Apr 10, 2018 at 6:31 PM, Andrey Ryabinin
> <aryabinin@virtuozzo.com> wrote:
>>
>>
>> On 04/10/2018 07:07 PM, Andrey Konovalov wrote:
>>> On Fri, Apr 6, 2018 at 2:27 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
>>>> On 04/06/2018 03:14 PM, Andrey Konovalov wrote:
>>>>> On Thu, Apr 5, 2018 at 3:02 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
>>>>>> 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 ignore
>>>> 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.
>>>
>>> Which pages do you mean by !PageSlab()?
>>
>> Literally the "PageSlab(page) == false" pages.
>>
>>> The ones that are allocated and freed by pagealloc, but mot managed by the slab allocator?
>>
>> Yes.
>>
>>> Perhaps we should then add tagging to the pagealloc hook instead?
>>>
>>
>> Of course the tagging would be in kasan_alloc_pages(), where else that could be? And instead of what?
> 
> I think I misunderstood your suggestion twice already :)
> 
> To make it clear, you're suggesting:
> 
> 1. Tag memory with a random tag in kasan_alloc_pages() and returned a
> tagged pointer from pagealloc.
 
Tag memory with a random tag in kasan_alloc_pages() and store that tag in page struct (that part is also in kasan_alloc_pages()).
page_address(page) will retrieve that tag from struct page to return tagged address.

I've no idea what do you mean by "returning a tagged pointer from pagealloc".
Once again, the page allocator (__alloc_pages_nodemask()) returns pointer to *struct page*,
not the address in the linear mapping where is that page mapped (or not mapped at all if this is highmem).
One have to call page_address()/kmap() to use that page.


> 2. Restore the tag for the pointers returned from page_address for
> !PageSlab() pages.
> 

Right.

> 3. Set the tag to 0xff for the pointers returned from page_address for
> PageSlab() pages.
> 

Right.

> Is this correct?
> 
> In 2 instead of storing the tag in page_struct, we can just recover it
> from the shadow memory that corresponds to that page. What do you
> think about this?

Sounds ok. Don't see any problem with that.