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.
