[PATCH] kasan: [v2]unpoison use memzero to init unaligned object

[PATCH] kasan: [v2]unpoison use memzero to init unaligned object

[yee.lee]

From: Yee Lee <yee.lee@mediatek.com>

Follows the discussion: https://patchwork.kernel.org/project/linux-mediatek/list/?series=504439

This patch Add memzero_explict to initialize unaligned object.

Based on the integrateion of initialization in kasan_unpoison(). The hwtag instructions, constrained with its granularity, has to overwrite the data btyes in unaligned objects. This would cause issue when it works with SLUB debug redzoning.

In this patch, an additional initalizaing path is added for the unaligned objects. It contains memzero_explict() to clear out the data and disables its init flag for the following hwtag actions.

In lab test, this path is executed about 1.1%(941/80854) within the overall kasan_unpoison during a non-debug booting process.

Lab test: QEMU5.2 (+mte) / linux kernel 5.13-rc7

Signed-off-by: Yee Lee <yee.lee@mediatek.com>
---
 mm/kasan/kasan.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index d8faa64614b7..edc11bcc3ff3 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -389,7 +389,7 @@ static inline void kasan_unpoison(const void *addr, size_t size, bool init)
 		return;
 	if (init && ((unsigned long)size & KASAN_GRANULE_MASK)) {
 		init = false;
-		memset((void *)addr, 0, size);
+		memzero_explicit((void *)addr, size);
 	}
 	size = round_up(size, KASAN_GRANULE_SIZE);
 	hw_set_mem_tag_range((void *)addr, size, tag, init);
2.18.0

Re: [PATCH] kasan: [v2]unpoison use memzero to init unaligned object

[Marco Elver]

On Tue, 22 Jun 2021 at 10:48, <yee.lee@mediatek.com> wrote:
>
> From: Yee Lee <yee.lee@mediatek.com>
>
> Follows the discussion: https://patchwork.kernel.org/project/linux-mediatek/list/?series=504439

The info about the percentage of how frequent this is could have been
provided as a simple reply to the discussion.

> This patch Add memzero_explict to initialize unaligned object.

This patch does not apply to anything (I see it depends on the previous patch).

What you need to do is modify the original patch, and then send a
[PATCH v2] (git helps with that by passing --reroll-count or -v) that
applies cleanly to your base kernel tree.

The commit message will usually end with '---' and then briefly denote
what changed since the last version.
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format

> Based on the integrateion of initialization in kasan_unpoison(). The hwtag instructions, constrained with its granularity, has to overwrite the data btyes in unaligned objects. This would cause issue when it works with SLUB debug redzoning.
>
> In this patch, an additional initalizaing path is added for the unaligned objects. It contains memzero_explict() to clear out the data and disables its init flag for the following hwtag actions.
>
> In lab test, this path is executed about 1.1%(941/80854) within the overall kasan_unpoison during a non-debug booting process.

Nice, thanks for the data. If it is somehow doable, however, I'd still
recommend to additionally guard the new code path by a check if
debug-support was requested. Ideally with an IS_ENABLED() config check
so that if it's a production kernel the branch is simply optimized out
by the compiler.

> Lab test: QEMU5.2 (+mte) / linux kernel 5.13-rc7
>
> Signed-off-by: Yee Lee <yee.lee@mediatek.com>
> ---
>  mm/kasan/kasan.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> index d8faa64614b7..edc11bcc3ff3 100644
> --- a/mm/kasan/kasan.h
> +++ b/mm/kasan/kasan.h
> @@ -389,7 +389,7 @@ static inline void kasan_unpoison(const void *addr, size_t size, bool init)
>                 return;
>         if (init && ((unsigned long)size & KASAN_GRANULE_MASK)) {
>                 init = false;
> -               memset((void *)addr, 0, size);
> +               memzero_explicit((void *)addr, size);
>         }
>         size = round_up(size, KASAN_GRANULE_SIZE);
>         hw_set_mem_tag_range((void *)addr, size, tag, init);
> 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 kasan-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20210622084723.27637-1-yee.lee%40mediatek.com.

Re: [PATCH] kasan: [v2]unpoison use memzero to init unaligned object

[Yee Lee]

On Tue, 2021-06-22 at 11:01 +0200, Marco Elver wrote:
> On Tue, 22 Jun 2021 at 10:48, <yee.lee@mediatek.com> wrote:
> > 
> > From: Yee Lee <yee.lee@mediatek.com>
> > 
> > Follows the discussion: 
> > https://patchwork.kernel.org/project/linux-mediatek/list/?series=504439
> 
> The info about the percentage of how frequent this is could have been
> provided as a simple reply to the discussion.
> 
> > This patch Add memzero_explict to initialize unaligned object.
> 
> This patch does not apply to anything (I see it depends on the
> previous patch).
> 
> What you need to do is modify the original patch, and then send a
> [PATCH v2] (git helps with that by passing --reroll-count or -v) that
> applies cleanly to your base kernel tree.
> 
> The commit message will usually end with '---' and then briefly
> denote
> what changed since the last version.
> 
Got it.

> 
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format
> 
> > Based on the integrateion of initialization in kasan_unpoison().
> > The hwtag instructions, constrained with its granularity, has to
> > overwrite the data btyes in unaligned objects. This would cause
> > issue when it works with SLUB debug redzoning.
> > 
> > In this patch, an additional initalizaing path is added for the
> > unaligned objects. It contains memzero_explict() to clear out the
> > data and disables its init flag for the following hwtag actions.
> > 
> > In lab test, this path is executed about 1.1%(941/80854) within the
> > overall kasan_unpoison during a non-debug booting process.
> 
> Nice, thanks for the data. If it is somehow doable, however, I'd
> still
> recommend to additionally guard the new code path by a check if
> debug-support was requested. Ideally with an IS_ENABLED() config
> check
> so that if it's a production kernel the branch is simply optimized
> out
> by the compiler.

Does it mean the memzero code path would be applied only at
CONFIG_DEBUG_SLUB enabled? It expects no other potential overwriting
in non-debug kernel.
 
By the way, based on de-coupling principle, adding a specific
conditional statement(is_enable slub_debug) in a primitive
funciton(kasan_unpoison) is not neat. It may be more proper that the
conditional statement be added in other procedures of slub alloc.
 
Thanks,

BR,
Yee

> 
> > Lab test: QEMU5.2 (+mte) / linux kernel 5.13-rc7
> > 
> > Signed-off-by: Yee Lee <yee.lee@mediatek.com>
> > ---
> >  mm/kasan/kasan.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> > index d8faa64614b7..edc11bcc3ff3 100644
> > --- a/mm/kasan/kasan.h
> > +++ b/mm/kasan/kasan.h
> > @@ -389,7 +389,7 @@ static inline void kasan_unpoison(const void
> > *addr, size_t size, bool init)
> >                 return;
> >         if (init && ((unsigned long)size & KASAN_GRANULE_MASK)) {
> >                 init = false;
> > -               memset((void *)addr, 0, size);
> > +               memzero_explicit((void *)addr, size);
> >         }
> >         size = round_up(size, KASAN_GRANULE_SIZE);
> >         hw_set_mem_tag_range((void *)addr, size, tag, init);
> > 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 kasan-dev+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit 
> > https://groups.google.com/d/msgid/kasan-dev/20210622084723.27637-1-yee.lee%40mediatek.com
> > .

Re: [PATCH] kasan: [v2]unpoison use memzero to init unaligned object

[Marco Elver]

On Tue, 22 Jun 2021 at 12:48, Yee Lee <yee.lee@mediatek.com> wrote:
>
> On Tue, 2021-06-22 at 11:01 +0200, Marco Elver wrote:
> > On Tue, 22 Jun 2021 at 10:48, <yee.lee@mediatek.com> wrote:
> > >
> > > From: Yee Lee <yee.lee@mediatek.com>
> > >
> > > Follows the discussion:
> > > https://patchwork.kernel.org/project/linux-mediatek/list/?series=504439
> >
> > The info about the percentage of how frequent this is could have been
> > provided as a simple reply to the discussion.
> >
> > > This patch Add memzero_explict to initialize unaligned object.
> >
> > This patch does not apply to anything (I see it depends on the
> > previous patch).
> >
> > What you need to do is modify the original patch, and then send a
> > [PATCH v2] (git helps with that by passing --reroll-count or -v) that
> > applies cleanly to your base kernel tree.
> >
> > The commit message will usually end with '---' and then briefly
> > denote
> > what changed since the last version.
> >
> Got it.
>
> >
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format
> >
> > > Based on the integrateion of initialization in kasan_unpoison().
> > > The hwtag instructions, constrained with its granularity, has to
> > > overwrite the data btyes in unaligned objects. This would cause
> > > issue when it works with SLUB debug redzoning.
> > >
> > > In this patch, an additional initalizaing path is added for the
> > > unaligned objects. It contains memzero_explict() to clear out the
> > > data and disables its init flag for the following hwtag actions.
> > >
> > > In lab test, this path is executed about 1.1%(941/80854) within the
> > > overall kasan_unpoison during a non-debug booting process.
> >
> > Nice, thanks for the data. If it is somehow doable, however, I'd
> > still
> > recommend to additionally guard the new code path by a check if
> > debug-support was requested. Ideally with an IS_ENABLED() config
> > check
> > so that if it's a production kernel the branch is simply optimized
> > out
> > by the compiler.
>
> Does it mean the memzero code path would be applied only at
> CONFIG_DEBUG_SLUB enabled? It expects no other potential overwriting
> in non-debug kernel.

Yes, if the problem only occurs with slub debugging enabled.

> By the way, based on de-coupling principle, adding a specific
> conditional statement(is_enable slub_debug) in a primitive
> funciton(kasan_unpoison) is not neat. It may be more proper that the
> conditional statement be added in other procedures of slub alloc.

What do you have in mind?

Well, there is kmem_cache_debug_flags(). Perhaps there's a better
place to add the check?

> Thanks,
>
> BR,
> Yee
>
> >
> > > Lab test: QEMU5.2 (+mte) / linux kernel 5.13-rc7
> > >
> > > Signed-off-by: Yee Lee <yee.lee@mediatek.com>
> > > ---
> > >  mm/kasan/kasan.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> > > index d8faa64614b7..edc11bcc3ff3 100644
> > > --- a/mm/kasan/kasan.h
> > > +++ b/mm/kasan/kasan.h
> > > @@ -389,7 +389,7 @@ static inline void kasan_unpoison(const void
> > > *addr, size_t size, bool init)
> > >                 return;
> > >         if (init && ((unsigned long)size & KASAN_GRANULE_MASK)) {
> > >                 init = false;
> > > -               memset((void *)addr, 0, size);
> > > +               memzero_explicit((void *)addr, size);
> > >         }
> > >         size = round_up(size, KASAN_GRANULE_SIZE);
> > >         hw_set_mem_tag_range((void *)addr, size, tag, init);
> > > 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 kasan-dev+unsubscribe@googlegroups.com.
> > > To view this discussion on the web visit
> > > https://groups.google.com/d/msgid/kasan-dev/20210622084723.27637-1-yee.lee%40mediatek.com
> > > .
>
> --
> 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 kasan-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/46b1468146206e6cef0c33ecbfd86e02ea819db4.camel%40mediatek.com.