Subject: [PATCH 0/2] kasan, mm: reset tag when access metadata

With hardware tag-based kasan enabled, we reset the tag
when we access metadata to avoid from false alarm.

Kuan-Ying Lee (2):
kasan, mm: reset tag when access metadata
kasan, mm: reset tag for hex dump address

mm/kmemleak.c | 6 +++---
mm/slub.c | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)

--
2.18.0


Subject: [PATCH 2/2] kasan, mm: reset tag for hex dump address

Text is a string. We need to move this kasan_reset_tag()
to address but text.

Signed-off-by: Kuan-Ying Lee <[email protected]>
---
mm/slub.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 6dad2b6fda6f..d20674f839ba 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -576,8 +576,8 @@ static void print_section(char *level, char *text, u8 *addr,
unsigned int length)
{
metadata_access_enable();
- print_hex_dump(level, kasan_reset_tag(text), DUMP_PREFIX_ADDRESS,
- 16, 1, addr, length, 1);
+ print_hex_dump(level, text, DUMP_PREFIX_ADDRESS,
+ 16, 1, kasan_reset_tag((void *)addr), length, 1);
metadata_access_disable();
}

--
2.18.0

2021-07-27 07:26:03

by Marco Elver

[permalink] [raw]
Subject: Re: [PATCH 2/2] kasan, mm: reset tag for hex dump address

On Tue, 27 Jul 2021 at 06:00, Kuan-Ying Lee <[email protected]> wrote:
>
> Text is a string. We need to move this kasan_reset_tag()
> to address but text.
>
> Signed-off-by: Kuan-Ying Lee <[email protected]>

This patch also makes sense (I think), thanks for sending. But it's
unclear what the problem is. The fact that when the address is printed
it still includes the tag? Or a false positive?
It'd be good to clarify in the commit message.

Here I'd also use a more descriptive patch title, something like
"kasan, slub: reset tag when printing address".

Also, I think this patch requires a:

Fixes: aa1ef4d7b3f6 ("kasan, mm: reset tags when accessing metadata")

So that stable kernels can pick this up if appropriate.

> ---
> mm/slub.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index 6dad2b6fda6f..d20674f839ba 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -576,8 +576,8 @@ static void print_section(char *level, char *text, u8 *addr,
> unsigned int length)
> {
> metadata_access_enable();
> - print_hex_dump(level, kasan_reset_tag(text), DUMP_PREFIX_ADDRESS,
> - 16, 1, addr, length, 1);
> + print_hex_dump(level, text, DUMP_PREFIX_ADDRESS,
> + 16, 1, kasan_reset_tag((void *)addr), length, 1);
> metadata_access_disable();
> }
>
> --
> 2.18.0
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20210727040021.21371-3-Kuan-Ying.Lee%40mediatek.com.

Subject: Re: [PATCH 2/2] kasan, mm: reset tag for hex dump address

On Tue, 2021-07-27 at 09:20 +0200, Marco Elver wrote:
> On Tue, 27 Jul 2021 at 06:00, Kuan-Ying Lee <
> [email protected]> wrote:
> >
> > Text is a string. We need to move this kasan_reset_tag()
> > to address but text.
> >
> > Signed-off-by: Kuan-Ying Lee <[email protected]>
>
> This patch also makes sense (I think), thanks for sending. But it's
> unclear what the problem is. The fact that when the address is
> printed
> it still includes the tag? Or a false positive?
> It'd be good to clarify in the commit message.

Yes, printed address includes the tag, so when we access the
metadata, we will encounter tag mismatch with HW tag-based kasan
enabled.

>
> Here I'd also use a more descriptive patch title, something like
> "kasan, slub: reset tag when printing address".
>
> Also, I think this patch requires a:
>
> Fixes: aa1ef4d7b3f6 ("kasan, mm: reset tags when accessing
> metadata")
>
> So that stable kernels can pick this up if appropriate.

Thank you, Marco.
I will refine commit message in the v2.

>
> > ---
> > mm/slub.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/slub.c b/mm/slub.c
> > index 6dad2b6fda6f..d20674f839ba 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -576,8 +576,8 @@ static void print_section(char *level, char
> > *text, u8 *addr,
> > unsigned int length)
> > {
> > metadata_access_enable();
> > - print_hex_dump(level, kasan_reset_tag(text),
> > DUMP_PREFIX_ADDRESS,
> > - 16, 1, addr, length, 1);
> > + print_hex_dump(level, text, DUMP_PREFIX_ADDRESS,
> > + 16, 1, kasan_reset_tag((void *)addr),
> > length, 1);
> > metadata_access_disable();
> > }
> >
> > --
> > 2.18.0
> >
> > --
> > You received this message because you are subscribed to the Google
> > Groups "kasan-dev" group.
> > To unsubscribe from this group and stop receiving emails from it,
> > send an email to [email protected].
> > To view this discussion on the web visit
> > https://urldefense.com/v3/__https://groups.google.com/d/msgid/kasan-dev/20210727040021.21371-3-Kuan-Ying.Lee*40mediatek.com__;JQ!!CTRNKA9wMg0ARbw!13XOuYbzPQrBvIDMNbrT7vm8RGc56Oqr402PDfQRDmHrrBsujrZUr7O9q24JeDJ_3NlWSQ$
> > .