[RFC] mm, slub: avoid zeroing kmalloc redzone

Peng Fan (OSS) posted 1 patch 1 year, 5 months ago
There is a newer version of this series
mm/slub.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
[RFC] mm, slub: avoid zeroing kmalloc redzone
Posted by Peng Fan (OSS) 1 year, 5 months ago
From: Peng Fan <peng.fan@nxp.com>

With "slub_debug=FUZ init_on_free=1 loglevel=7" set in bootargs and
CONFIG_SLAB_FREELIST_HARDENED is set. There is kernel dump:
[    0.000000] =============================================================================
[    0.000000] BUG kmalloc-8 (Not tainted): kmalloc Redzone overwritten
[    0.000000] -----------------------------------------------------------------------------
[    0.000000]
[    0.000000] 0xffff000010032858-0xffff00001003285f @offset=2136. First byte 0x0 instead of 0xcc
[    0.000000] FIX kmalloc-8: Restoring kmalloc Redzone 0xffff000010032858-0xffff00001003285f=0xcc
[    0.000000] Slab 0xfffffdffc0400c80 objects=36 used=23 fp=0xffff000010032a18 flags=0x3fffe0000000200(workingset|node=0|zone=0|lastcpupid=0x1ffff)
[    0.000000] Object 0xffff000010032858 @offset=2136 fp=0xffff0000100328c8
[    0.000000]
[    0.000000] Redzone  ffff000010032850: cc cc cc cc cc cc cc cc                          ........
[    0.000000] Object   ffff000010032858: cc cc cc cc cc cc cc cc                          ........
[    0.000000] Redzone  ffff000010032860: cc cc cc cc cc cc cc cc                          ........
[    0.000000] Padding  ffff0000100328b4: 00 00 00 00 00 00 00 00 00 00 00 00              ............
[    0.000000] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 6.11.0-rc3-next-20240814-00004-g61844c55c3f4 #144
[    0.000000] Hardware name: NXP i.MX95 19X19 board (DT)
[    0.000000] Call trace:
[    0.000000]  dump_backtrace+0x90/0xe8
[    0.000000]  show_stack+0x18/0x24
[    0.000000]  dump_stack_lvl+0x74/0x8c
[    0.000000]  dump_stack+0x18/0x24
[    0.000000]  print_trailer+0x150/0x218
[    0.000000]  check_object+0xe4/0x454
[    0.000000]  free_to_partial_list+0x2f8/0x5ec

It is because the kmalloc redzone area is cleared or orig_size is
cleared. When s->object_size is larger than orig_size, just clear the
orig_size area. And restore the value of orig_size.

Fixes: d57a964e09c2 ("kasan, mm: integrate slab init_on_free with HW_TAGS")
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 mm/slub.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 94f5a4143825..d03957d15bbf 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2282,14 +2282,22 @@ bool slab_free_hook(struct kmem_cache *s, void *x, bool init,
 	 */
 	if (unlikely(init)) {
 		int rsize;
-		unsigned int inuse;
+		unsigned int inuse, orig_size;
 
 		inuse = get_info_end(s);
+		orig_size = get_orig_size(s, x);
 		if (!kasan_has_integrated_init())
-			memset(kasan_reset_tag(x), 0, s->object_size);
+			memset(kasan_reset_tag(x), 0,
+			       s->object_size > orig_size ? orig_size : s->object_size);
 		rsize = (s->flags & SLAB_RED_ZONE) ? s->red_left_pad : 0;
 		memset((char *)kasan_reset_tag(x) + inuse, 0,
 		       s->size - inuse - rsize);
+		/*
+		 * Restore orig_size, otherwize kmalloc redzone overwritten
+		 * would be reported
+		 */
+		set_orig_size(s, x, orig_size);
+
 	}
 	/* KASAN might put x into memory quarantine, delaying its reuse. */
 	return !kasan_slab_free(s, x, init, still_accessible);
-- 
2.37.1
Re: [RFC] mm, slub: avoid zeroing kmalloc redzone
Posted by Hyeonggon Yoo 1 year, 5 months ago
On Mon, Aug 19, 2024 at 3:32 PM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote:
>
> From: Peng Fan <peng.fan@nxp.com>
>
> With "slub_debug=FUZ init_on_free=1 loglevel=7" set in bootargs and
> CONFIG_SLAB_FREELIST_HARDENED is set. There is kernel dump:
> [    0.000000] =============================================================================
> [    0.000000] BUG kmalloc-8 (Not tainted): kmalloc Redzone overwritten
> [    0.000000] -----------------------------------------------------------------------------
> [    0.000000]
> [    0.000000] 0xffff000010032858-0xffff00001003285f @offset=2136. First byte 0x0 instead of 0xcc
> [    0.000000] FIX kmalloc-8: Restoring kmalloc Redzone 0xffff000010032858-0xffff00001003285f=0xcc
> [    0.000000] Slab 0xfffffdffc0400c80 objects=36 used=23 fp=0xffff000010032a18 flags=0x3fffe0000000200(workingset|node=0|zone=0|lastcpupid=0x1ffff)
> [    0.000000] Object 0xffff000010032858 @offset=2136 fp=0xffff0000100328c8
> [    0.000000]
> [    0.000000] Redzone  ffff000010032850: cc cc cc cc cc cc cc cc                          ........
> [    0.000000] Object   ffff000010032858: cc cc cc cc cc cc cc cc                          ........
> [    0.000000] Redzone  ffff000010032860: cc cc cc cc cc cc cc cc                          ........
> [    0.000000] Padding  ffff0000100328b4: 00 00 00 00 00 00 00 00 00 00 00 00              ............
> [    0.000000] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 6.11.0-rc3-next-20240814-00004-g61844c55c3f4 #144
> [    0.000000] Hardware name: NXP i.MX95 19X19 board (DT)
> [    0.000000] Call trace:
> [    0.000000]  dump_backtrace+0x90/0xe8
> [    0.000000]  show_stack+0x18/0x24
> [    0.000000]  dump_stack_lvl+0x74/0x8c
> [    0.000000]  dump_stack+0x18/0x24
> [    0.000000]  print_trailer+0x150/0x218
> [    0.000000]  check_object+0xe4/0x454
> [    0.000000]  free_to_partial_list+0x2f8/0x5ec
>
> It is because the kmalloc redzone area is cleared or orig_size is
> cleared.

Setting orig_size treats the wasted space (object_size - orig_size) as
redzones. (in check_object())
When orig_size is set to zero, the entire object is perceived as a redzone.
Could you elaborate the explanation in the description?

> When s->object_size is larger than orig_size, just clear the
> orig_size area. And restore the value of orig_size.
>
> Fixes: d57a964e09c2 ("kasan, mm: integrate slab init_on_free with HW_TAGS")

I think the proper 'Fixes' commit should be 946fa0dbf2d8 ("mm/slub:
extend redzone check to extra allocated kmalloc space than requested")
because it is the commit that extends redzone check, but did not
address init_on_free=1 case.

> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  mm/slub.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index 94f5a4143825..d03957d15bbf 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -2282,14 +2282,22 @@ bool slab_free_hook(struct kmem_cache *s, void *x, bool init,
>          */
>         if (unlikely(init)) {
>                 int rsize;
> -               unsigned int inuse;
> +               unsigned int inuse, orig_size;
>
>                 inuse = get_info_end(s);
> +               orig_size = get_orig_size(s, x);
>                 if (!kasan_has_integrated_init())
> -                       memset(kasan_reset_tag(x), 0, s->object_size);
> +                       memset(kasan_reset_tag(x), 0,
> +                              s->object_size > orig_size ? orig_size : s->object_size);

the size can simply be orig_size, as orig_size returns object_size
when it is not enabled
and orig_size can never be bigger than object_size.

>                 rsize = (s->flags & SLAB_RED_ZONE) ? s->red_left_pad : 0;
>                 memset((char *)kasan_reset_tag(x) + inuse, 0,
>                        s->size - inuse - rsize);
> +               /*
> +                * Restore orig_size, otherwize kmalloc redzone overwritten
> +                * would be reported
> +                */
> +               set_orig_size(s, x, orig_size);