* [PATCH V1] mm, slub: avoid zeroing kmalloc redzone
@ 2024-08-23 6:24 Peng Fan (OSS)
2024-08-25 13:05 ` Feng Tang
0 siblings, 1 reply; 5+ messages in thread
From: Peng Fan (OSS) @ 2024-08-23 6:24 UTC (permalink / raw)
To: nicolas.bouchinet, chengming.zhou, vbabka, Christoph Lameter,
Pekka Enberg, David Rientjes, Joonsoo Kim, Andrew Morton,
Roman Gushchin, Hyeonggon Yoo, Feng Tang,
open list:SLAB ALLOCATOR, open list
Cc: Peng Fan
From: Peng Fan <peng.fan@nxp.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:
[ 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")
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
RFC->V1:
Update commit log (Per Hyeonggon)
Use orig_size to do memset(Per Hyeonggon)
Add get_orig_size and set_orig_size when CONFIG_SLUB_DEBUG not enabled(kernel test robot)
https://lore.kernel.org/all/20240819064115.385086-1-peng.fan@oss.nxp.com/
mm/slub.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index 94f5a4143825..a5fbeb2835b1 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1895,6 +1895,15 @@ static inline void inc_slabs_node(struct kmem_cache *s, int node,
static inline void dec_slabs_node(struct kmem_cache *s, int node,
int objects) {}
+static inline unsigned int get_orig_size(struct kmem_cache *s, void *object)
+{
+ return s->object_size;
+}
+
+static inline void set_orig_size(struct kmem_cache *s, void *object,
+ unsigned int orig_size)
+{}
+
#ifndef CONFIG_SLUB_TINY
static bool freelist_corrupted(struct kmem_cache *s, struct slab *slab,
void **freelist, void *nextfree)
@@ -2282,14 +2291,21 @@ 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, orig_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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH V1] mm, slub: avoid zeroing kmalloc redzone
2024-08-23 6:24 [PATCH V1] mm, slub: avoid zeroing kmalloc redzone Peng Fan (OSS)
@ 2024-08-25 13:05 ` Feng Tang
2024-08-28 2:43 ` Peng Fan
2024-08-28 16:53 ` Vlastimil Babka
0 siblings, 2 replies; 5+ messages in thread
From: Feng Tang @ 2024-08-25 13:05 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: nicolas.bouchinet, chengming.zhou, vbabka, Christoph Lameter,
Pekka Enberg, David Rientjes, Joonsoo Kim, Andrew Morton,
Roman Gushchin, Hyeonggon Yoo, open list:SLAB ALLOCATOR,
open list, Peng Fan
On Fri, Aug 23, 2024 at 02:24:15PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.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:
>
> [ 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@intel.com>
with one small nit below
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>
> RFC->V1:
> Update commit log (Per Hyeonggon)
> Use orig_size to do memset(Per Hyeonggon)
> Add get_orig_size and set_orig_size when CONFIG_SLUB_DEBUG not enabled(kernel test robot)
> https://lore.kernel.org/all/20240819064115.385086-1-peng.fan@oss.nxp.com/
>
> mm/slub.c | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index 94f5a4143825..a5fbeb2835b1 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1895,6 +1895,15 @@ static inline void inc_slabs_node(struct kmem_cache *s, int node,
> static inline void dec_slabs_node(struct kmem_cache *s, int node,
> int objects) {}
>
> +static inline unsigned int get_orig_size(struct kmem_cache *s, void *object)
> +{
> + return s->object_size;
> +}
> +
> +static inline void set_orig_size(struct kmem_cache *s, void *object,
> + unsigned int orig_size)
> +{}
Current get_orig_size() and set_orig_size() are protected by
CONFIG_SLUB_DEUG=y macro, and with this patch, they will be called
in both ON and OFF case. Maybe we can just lift those existing
functions out of the "#ifdef CONFIG_SLUB_DEBUG" protection?
Thanks,
Feng
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [PATCH V1] mm, slub: avoid zeroing kmalloc redzone
2024-08-25 13:05 ` Feng Tang
@ 2024-08-28 2:43 ` Peng Fan
2024-08-28 16:53 ` Vlastimil Babka
1 sibling, 0 replies; 5+ messages in thread
From: Peng Fan @ 2024-08-28 2:43 UTC (permalink / raw)
To: Feng Tang, Peng Fan (OSS)
Cc: nicolas.bouchinet@clip-os.org, chengming.zhou@linux.dev,
vbabka@suse.cz, Christoph Lameter, Pekka Enberg, David Rientjes,
Joonsoo Kim, Andrew Morton, Roman Gushchin, Hyeonggon Yoo,
open list:SLAB ALLOCATOR, open list
> Subject: Re: [PATCH V1] mm, slub: avoid zeroing kmalloc redzone
>
> On Fri, Aug 23, 2024 at 02:24:15PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.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:
> >
> > [ 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
> >
> > 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
> > s->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@intel.com>
>
> with one small nit below
>
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >
> > RFC->V1:
> > Update commit log (Per Hyeonggon)
> > Use orig_size to do memset(Per Hyeonggon) Add get_orig_size and
> > set_orig_size when CONFIG_SLUB_DEBUG not enabled(kernel test
> robot)
> >
> >
> >
> > mm/slub.c | 20 ++++++++++++++++++--
> > 1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/slub.c b/mm/slub.c
> > index 94f5a4143825..a5fbeb2835b1 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -1895,6 +1895,15 @@ static inline void inc_slabs_node(struct
> > kmem_cache *s, int node, static inline void dec_slabs_node(struct
> kmem_cache *s, int node,
> > int objects)
> {}
> >
> > +static inline unsigned int get_orig_size(struct kmem_cache *s, void
> > +*object) {
> > + return s->object_size;
> > +}
> > +
> > +static inline void set_orig_size(struct kmem_cache *s, void *object,
> > + unsigned int orig_size)
> > +{}
>
> Current get_orig_size() and set_orig_size() are protected by
> CONFIG_SLUB_DEUG=y macro, and with this patch, they will be called
> in both ON and OFF case. Maybe we can just lift those existing
> functions out of the "#ifdef CONFIG_SLUB_DEBUG" protection?
This will build a bit more code when CONFIG_SLUB_DEBUG is
not defined. But it should be fine. I will wait to see if other people
have comments on this.
Thanks,
Peng.
>
> Thanks,
> Feng
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH V1] mm, slub: avoid zeroing kmalloc redzone
2024-08-25 13:05 ` Feng Tang
2024-08-28 2:43 ` Peng Fan
@ 2024-08-28 16:53 ` Vlastimil Babka
2024-08-29 5:16 ` Feng Tang
1 sibling, 1 reply; 5+ messages in thread
From: Vlastimil Babka @ 2024-08-28 16:53 UTC (permalink / raw)
To: Feng Tang, Peng Fan (OSS)
Cc: nicolas.bouchinet, chengming.zhou, Christoph Lameter,
Pekka Enberg, David Rientjes, Joonsoo Kim, Andrew Morton,
Roman Gushchin, Hyeonggon Yoo, open list:SLAB ALLOCATOR,
open list, Peng Fan
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@nxp.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@intel.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/
> with one small nit below
>
>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> ---
>>
>> RFC->V1:
>> Update commit log (Per Hyeonggon)
>> Use orig_size to do memset(Per Hyeonggon)
>> Add get_orig_size and set_orig_size when CONFIG_SLUB_DEBUG not enabled(kernel test robot)
>> https://lore.kernel.org/all/20240819064115.385086-1-peng.fan@oss.nxp.com/
>>
>> mm/slub.c | 20 ++++++++++++++++++--
>> 1 file changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/slub.c b/mm/slub.c
>> index 94f5a4143825..a5fbeb2835b1 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
>> @@ -1895,6 +1895,15 @@ static inline void inc_slabs_node(struct kmem_cache *s, int node,
>> static inline void dec_slabs_node(struct kmem_cache *s, int node,
>> int objects) {}
>>
>> +static inline unsigned int get_orig_size(struct kmem_cache *s, void *object)
>> +{
>> + return s->object_size;
>> +}
>> +
>> +static inline void set_orig_size(struct kmem_cache *s, void *object,
>> + unsigned int orig_size)
>> +{}
>
> Current get_orig_size() and set_orig_size() are protected by
> CONFIG_SLUB_DEUG=y macro, and with this patch, they will be called
> in both ON and OFF case. Maybe we can just lift those existing
> functions out of the "#ifdef CONFIG_SLUB_DEBUG" protection?
I agree. As for the "code is larger for !CONFIG_SLUB_DEBUG" concern I think
the code will be eliminated anyway, because of the
if (!slub_debug_orig_size(s))
return;
and slub_debug_orig_size() does kmem_cache_debug_flags() which is a
compile-time false for !CONFIG_SLUB_DEBUG.
> Thanks,
> Feng
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH V1] mm, slub: avoid zeroing kmalloc redzone
2024-08-28 16:53 ` Vlastimil Babka
@ 2024-08-29 5:16 ` Feng Tang
0 siblings, 0 replies; 5+ messages in thread
From: Feng Tang @ 2024-08-29 5:16 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Peng Fan (OSS), nicolas.bouchinet, chengming.zhou,
Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
Andrew Morton, Roman Gushchin, Hyeonggon Yoo,
open list:SLAB ALLOCATOR, open list, Peng Fan
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@nxp.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@intel.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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-08-29 5:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-23 6:24 [PATCH V1] mm, slub: avoid zeroing kmalloc redzone Peng Fan (OSS)
2024-08-25 13:05 ` Feng Tang
2024-08-28 2:43 ` Peng Fan
2024-08-28 16:53 ` Vlastimil Babka
2024-08-29 5:16 ` Feng Tang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).