| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID:
<PAXPR04MB845979EA35B856D10A017BEA888C2@PAXPR04MB8459.eurprd04.prod.outlook.com>
Date: Mon, 19 Aug 2024 09:39:16 +0000
From: Peng Fan <peng.fan@....com>
To: Hyeonggon Yoo <42.hyeyoo@...il.com>, "Peng Fan (OSS)"
<peng.fan@....nxp.com>
CC: "nicolas.bouchinet@...p-os.org" <nicolas.bouchinet@...p-os.org>,
"chengming.zhou@...ux.dev" <chengming.zhou@...ux.dev>, "vbabka@...e.cz"
<vbabka@...e.cz>, Christoph Lameter <cl@...ux.com>, Pekka Enberg
<penberg@...nel.org>, David Rientjes <rientjes@...gle.com>, Joonsoo Kim
<iamjoonsoo.kim@....com>, Andrew Morton <akpm@...ux-foundation.org>, Roman
Gushchin <roman.gushchin@...ux.dev>, Marco Elver <elver@...gle.com>, Andrey
Konovalov <andreyknvl@...il.com>, "open list:SLAB ALLOCATOR"
<linux-mm@...ck.org>, open list <linux-kernel@...r.kernel.org>
Subject: RE: [RFC] mm, slub: avoid zeroing kmalloc redzone
Hi Hyeonggon,
Thanks for your quick reply.
> Subject: Re: [RFC] mm, slub: avoid zeroing kmalloc redzone
>
> On Mon, Aug 19, 2024 at 3:32 PM Peng Fan (OSS)
> <peng.fan@....nxp.com> wrote:
> >
> > From: Peng Fan <peng.fan@....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=0x1f
> fff)
> > [ 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?
Quote your reply here. Hope the following is good:
"
With commit 946fa0dbf2d8
("mm/slub: extend redzone check to extra allocated kmalloc space than requested"),
setting orig_size treats the wasted space (object_size - orig_size) as
redzones. But (in check_object()) When orig_size is set to zero, the entire
object is perceived as a redzone. To a valid allocated kmalloc space,
when init_on_free=1, the wasted space and the orig_size should
not be cleared to 0, otherwise there will be kernel dump:
[kernel dump here]
To address the issue, use orig_size to clear the used area. And restore
the value of orig_size after clear the remaining area.
"
>
> > 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.
Yeah. I will correct in v2.
>
> > Signed-off-by: Peng Fan <peng.fan@....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.
Fix in v2: "memset(kasan_reset_tag(x), 0, orig);"
>
> > 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);
Thanks for help reviewing.
Thanks,
Peng.
Powered by blists - more mailing lists