On 04/06/2018 03:14 PM, Andrey Konovalov wrote: > On Thu, Apr 5, 2018 at 3:02 PM, Andrey Ryabinin <aryabinin@xxxxxxxxxxxxx> wrote: >> On 04/04/2018 08:00 PM, Andrey Konovalov wrote: >>> On Wed, Apr 4, 2018 at 2:39 PM, Andrey Ryabinin <aryabinin@xxxxxxxxxxxxx> 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 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.
