linux-kernel - Re: [PATCH V1] mm, slub: avoid zeroing kmalloc redzone

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZtAEF0WLfMr8LYLL@feng-clx.sh.intel.com>
Date: Thu, 29 Aug 2024 13:16:07 +0800
From: Feng Tang <feng.tang@...el.com>
To: Vlastimil Babka <vbabka@...e.cz>
CC: "Peng Fan (OSS)" <peng.fan@....nxp.com>, <nicolas.bouchinet@...p-os.org>,
	<chengming.zhou@...ux.dev>, 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>, Hyeonggon Yoo <42.hyeyoo@...il.com>,
	"open list:SLAB ALLOCATOR" <linux-mm@...ck.org>, open list
	<linux-kernel@...r.kernel.org>, Peng Fan <peng.fan@....com>
Subject: Re: [PATCH V1] mm, slub: avoid zeroing kmalloc redzone

On Wed, Aug 28, 2024 at 06:53:11PM +0200, Vlastimil Babka wrote:
> On 8/25/24 15:05, Feng Tang wrote:
> > On Fri, Aug 23, 2024 at 02:24:15PM +0800, Peng Fan (OSS) wrote:
> >> From: Peng Fan <peng.fan@....com>
> >> 
> >> 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:
> 
> Could we make it more clear?
> 
> Since 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 a redzone. However with init_on_free=1 we clear
> the full object->size, including the redzone.
> 
> Additionally we clear the object metadata, including the stored orig_size,
> making it zero, which makes check_object() treat the the whole object as a
> redzone.
> 
> These issues lead to the following BUG report with "slub_debug=FUZ
> init_on_free=1":
> 
> >> 
> >> [    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
> >> 
> >> 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 CONFIG_SLUB_DEBUG not defined, (get_orig_size()' directly returns
> >> s->object_size. So when using memset to init the area, the size can simply
> >> be orig_size, as orig_size returns object_size when CONFIG_SLUB_DEBUG not
> >> enabled. And orig_size can never be bigger than object_size.
> >> 
> >> Fixes: 946fa0dbf2d8 ("mm/slub: extend redzone check to extra allocated kmalloc space than requested")
> > 
> > Thanks for the fix! I missed to test the 'init_on_free' case back then.
> > 
> > Reviewed-by: Feng Tang <feng.tang@...el.com>
> 
> By the way Feng, have you noticed the other issue we have, with krealloc()
> and redzoning? Want to look at that? Thanks.
> 
> https://lore.kernel.org/all/44fa564b-9c8f-4ac2-bce3-f6d2c99b73b7@suse.cz/

Sure. Will check that thread (it might take a while as I just came back
from vacation). thanks

- Feng