| Message ID | cca947c05c4881cf5b7548614909f1625f47be61.1638825394.git.andreyknvl@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | kasan, vmalloc, arm64: add vmalloc tagging support for SW/HW_TAGS | expand |
On Mon, Dec 06, 2021 at 10:43:45PM +0100, andrey.konovalov@linux.dev wrote: > From: Andrey Konovalov <andreyknvl@google.com> > > __GFP_ZEROTAGS should only be effective if memory is being zeroed. > Currently, hardware tag-based KASAN violates this requirement. > > Fix by including an initialization check along with checking for > __GFP_ZEROTAGS. > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > Reviewed-by: Alexander Potapenko <glider@google.com> > --- > mm/kasan/hw_tags.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c > index 0b8225add2e4..c643740b8599 100644 > --- a/mm/kasan/hw_tags.c > +++ b/mm/kasan/hw_tags.c > @@ -199,11 +199,12 @@ void kasan_alloc_pages(struct page *page, unsigned int order, gfp_t flags) > * page_alloc.c. > */ > bool init = !want_init_on_free() && want_init_on_alloc(flags); > + bool init_tags = init && (flags & __GFP_ZEROTAGS); > > if (flags & __GFP_SKIP_KASAN_POISON) > SetPageSkipKASanPoison(page); > > - if (flags & __GFP_ZEROTAGS) { > + if (init_tags) { You can probably leave this unchanged but add a WARN_ON_ONCE() if !init. AFAICT there's only a single place where __GFP_ZEROTAGS is passed.
On Fri, Dec 10, 2021 at 6:48 PM Catalin Marinas <catalin.marinas@arm.com> wrote: > > On Mon, Dec 06, 2021 at 10:43:45PM +0100, andrey.konovalov@linux.dev wrote: > > From: Andrey Konovalov <andreyknvl@google.com> > > > > __GFP_ZEROTAGS should only be effective if memory is being zeroed. > > Currently, hardware tag-based KASAN violates this requirement. > > > > Fix by including an initialization check along with checking for > > __GFP_ZEROTAGS. > > > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > > Reviewed-by: Alexander Potapenko <glider@google.com> > > --- > > mm/kasan/hw_tags.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c > > index 0b8225add2e4..c643740b8599 100644 > > --- a/mm/kasan/hw_tags.c > > +++ b/mm/kasan/hw_tags.c > > @@ -199,11 +199,12 @@ void kasan_alloc_pages(struct page *page, unsigned int order, gfp_t flags) > > * page_alloc.c. > > */ > > bool init = !want_init_on_free() && want_init_on_alloc(flags); > > + bool init_tags = init && (flags & __GFP_ZEROTAGS); > > > > if (flags & __GFP_SKIP_KASAN_POISON) > > SetPageSkipKASanPoison(page); > > > > - if (flags & __GFP_ZEROTAGS) { > > + if (init_tags) { > > You can probably leave this unchanged but add a WARN_ON_ONCE() if !init. > AFAICT there's only a single place where __GFP_ZEROTAGS is passed. Yes, there's only one such place. In a later patch, I implement handling __GFP_ZEROTAGS in regardless of having __GFP_ZERO present or not, so adding WARN_ON() here and then removing it probably doesn't make much sense. As per what you said in the other message, I've left this unchanged. Thanks!
diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c index 0b8225add2e4..c643740b8599 100644 --- a/mm/kasan/hw_tags.c +++ b/mm/kasan/hw_tags.c @@ -199,11 +199,12 @@ void kasan_alloc_pages(struct page *page, unsigned int order, gfp_t flags) * page_alloc.c. */ bool init = !want_init_on_free() && want_init_on_alloc(flags); + bool init_tags = init && (flags & __GFP_ZEROTAGS); if (flags & __GFP_SKIP_KASAN_POISON) SetPageSkipKASanPoison(page); - if (flags & __GFP_ZEROTAGS) { + if (init_tags) { int i; for (i = 0; i != 1 << order; ++i)