On Wed, May 25, 2022 at 5:45 PM Catalin Marinas <catalin.marinas@xxxxxxx> wrote: > > > Does this have to be GFP_USER? Can we add new flags to > > GFP_HIGHUSER_MOVABLE instead? > > > > For instance, Peter added __GFP_SKIP_KASAN_POISON to > > GFP_HIGHUSER_MOVABLE in c275c5c6d50a0. > > The above commit was a performance improvement. Here we need to address > the correctness. However, looking through the GFP_USER cases, I don't > think any of them is at risk of ending up in user space with PROT_MTE. > There are places where GFP_USER is passed to kmalloc() for in-kernel > objects that would never be mapped to user, though the new gfp flag > won't be taken into account. Yeah, those kmalloc()'s look suspicious. > I'm ok to move the new flag to the GFP_HIGHUSER_MOVABLE but probably > still keep a page_kasan_tag_reset() on the set_pte_at() path together > with a WARN_ON_ONCE() if we miss anything. GFP_HIGHUSER_MOVABLE is used in fewer places than GFP_USER, so if it works - great! However, see below. > > Adding __GFP_SKIP_KASAN_UNPOISON makes sense, but we still need to > > reset the tag in page->flags. > > My thought was to reset the tag in page->flags based on 'unpoison' > alone without any extra flags. We use this flag for vmalloc() pages but > it seems we don't reset the page tags (as we do via > kasan_poison_slab()). I just realized that we already have __GFP_ZEROTAGS that initializes both in-memory and page->flags tags. Currently only used for user pages allocated via alloc_zeroed_user_highpage_movable(). Perhaps we can add this flag to GFP_HIGHUSER_MOVABLE? We'll also need to change the behavior of __GFP_ZEROTAGS to work even when GFP_ZERO is not set, but this doesn't seem to be a problem. And, at this point, we can probably combine __GFP_ZEROTAGS with __GFP_SKIP_KASAN_POISON, as they both would target user pages. Does this make sense?
