From: Andrey Konovalov <andreyknvl@gmail.com> To: Catalin Marinas <catalin.marinas@arm.com> Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com>, Will Deacon <will@kernel.org>, Vincenzo Frascino <vincenzo.frascino@arm.com>, Peter Collingbourne <pcc@google.com>, kasan-dev <kasan-dev@googlegroups.com>, Linux Memory Management List <linux-mm@kvack.org>, Linux ARM <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH 0/3] kasan: Fix ordering between MTE tag colouring and page->flags Date: Wed, 25 May 2022 19:41:08 +0200 [thread overview] Message-ID: <CA+fCnZc1CUatXbp=KVSD3s71k1GcoPdNCFF1rSxfyPaY4e0qaQ@mail.gmail.com> (raw) In-Reply-To: <Yo5PAJTI7CwxVZ/q@arm.com> On Wed, May 25, 2022 at 5:45 PM Catalin Marinas <catalin.marinas@arm.com> 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?
WARNING: multiple messages have this Message-ID (diff)
From: Andrey Konovalov <andreyknvl@gmail.com> To: Catalin Marinas <catalin.marinas@arm.com> Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com>, Will Deacon <will@kernel.org>, Vincenzo Frascino <vincenzo.frascino@arm.com>, Peter Collingbourne <pcc@google.com>, kasan-dev <kasan-dev@googlegroups.com>, Linux Memory Management List <linux-mm@kvack.org>, Linux ARM <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH 0/3] kasan: Fix ordering between MTE tag colouring and page->flags Date: Wed, 25 May 2022 19:41:08 +0200 [thread overview] Message-ID: <CA+fCnZc1CUatXbp=KVSD3s71k1GcoPdNCFF1rSxfyPaY4e0qaQ@mail.gmail.com> (raw) In-Reply-To: <Yo5PAJTI7CwxVZ/q@arm.com> On Wed, May 25, 2022 at 5:45 PM Catalin Marinas <catalin.marinas@arm.com> 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? _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-05-25 17:41 UTC|newest] Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-05-17 18:09 [PATCH 0/3] kasan: Fix ordering between MTE tag colouring and page->flags Catalin Marinas 2022-05-17 18:09 ` Catalin Marinas 2022-05-17 18:09 ` [PATCH 1/3] mm: kasan: Ensure the tags are visible before the tag in page->flags Catalin Marinas 2022-05-17 18:09 ` Catalin Marinas 2022-05-21 22:14 ` Andrey Konovalov 2022-05-21 22:14 ` Andrey Konovalov 2022-05-17 18:09 ` [PATCH 2/3] mm: kasan: Reset the tag on pages intended for user Catalin Marinas 2022-05-17 18:09 ` Catalin Marinas 2022-05-21 22:15 ` Andrey Konovalov 2022-05-21 22:15 ` Andrey Konovalov 2022-05-17 18:09 ` [PATCH 3/3] arm64: kasan: Revert "arm64: mte: reset the page tag in page->flags" Catalin Marinas 2022-05-17 18:09 ` Catalin Marinas 2022-05-21 22:16 ` Andrey Konovalov 2022-05-21 22:16 ` Andrey Konovalov 2022-05-19 21:45 ` [PATCH 0/3] kasan: Fix ordering between MTE tag colouring and page->flags Andrey Konovalov 2022-05-19 21:45 ` Andrey Konovalov 2022-05-20 13:01 ` Catalin Marinas 2022-05-20 13:01 ` Catalin Marinas 2022-05-21 22:20 ` Andrey Konovalov 2022-05-21 22:20 ` Andrey Konovalov 2022-05-25 15:45 ` Catalin Marinas 2022-05-25 15:45 ` Catalin Marinas 2022-05-25 17:41 ` Andrey Konovalov [this message] 2022-05-25 17:41 ` Andrey Konovalov 2022-05-26 12:24 ` Catalin Marinas 2022-05-26 12:24 ` Catalin Marinas 2022-05-31 17:16 ` Andrey Konovalov 2022-05-31 17:16 ` Andrey Konovalov 2022-06-09 18:32 ` Catalin Marinas 2022-06-09 18:32 ` Catalin Marinas 2022-06-09 18:40 ` Andrey Konovalov 2022-06-09 18:40 ` Andrey Konovalov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CA+fCnZc1CUatXbp=KVSD3s71k1GcoPdNCFF1rSxfyPaY4e0qaQ@mail.gmail.com' \ --to=andreyknvl@gmail.com \ --cc=catalin.marinas@arm.com \ --cc=kasan-dev@googlegroups.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-mm@kvack.org \ --cc=pcc@google.com \ --cc=ryabinin.a.a@gmail.com \ --cc=vincenzo.frascino@arm.com \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.