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.
