Re: [PATCH] tracing: Inform kmemleak of saved_cmdlines allocation

2024-02-19 Thread Catalin Marinas
On Wed, Feb 14, 2024 at 10:26:14AM -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" 
> 
> The allocation of the struct saved_cmdlines_buffer structure changed from:
> 
> s = kmalloc(sizeof(*s), GFP_KERNEL);
>   s->saved_cmdlines = kmalloc_array(TASK_COMM_LEN, val, GFP_KERNEL);
> 
> to:
> 
>   orig_size = sizeof(*s) + val * TASK_COMM_LEN;
>   order = get_order(orig_size);
>   size = 1 << (order + PAGE_SHIFT);
>   page = alloc_pages(GFP_KERNEL, order);
>   if (!page)
>   return NULL;
> 
>   s = page_address(page);
>   memset(s, 0, sizeof(*s));
> 
>   s->saved_cmdlines = kmalloc_array(TASK_COMM_LEN, val, GFP_KERNEL);
> 
> Where that s->saved_cmdlines allocation looks to be a dangling allocation
> to kmemleak. That's because kmemleak only keeps track of kmalloc()
> allocations. For allocations that use page_alloc() directly, the kmemleak
> needs to be explicitly informed about it.
> 
> Add kmemleak_alloc() and kmemleak_free() around the page allocation so
> that it doesn't give the following false positive:
> 
> unreferenced object 0x8881010c8000 (size 32760):
>   comm "swapper", pid 0, jiffies 4294667296
>   hex dump (first 32 bytes):
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  
>   backtrace (crc ae6ec1b9):
> [] kmemleak_alloc+0x45/0x80
> [] __kmalloc_large_node+0x10d/0x190
> [] __kmalloc+0x3b1/0x4c0
> [] allocate_cmdlines_buffer+0x113/0x230
> [] tracer_alloc_buffers.isra.0+0x124/0x460
> [] early_trace_init+0x14/0xa0
> [] start_kernel+0x12e/0x3c0
> [] x86_64_start_reservations+0x18/0x30
> [] x86_64_start_kernel+0x7b/0x80
> [] secondary_startup_64_no_verify+0x15e/0x16b
> 
> Link: https://lore.kernel.org/linux-trace-kernel/87r0hfnr9r@kernel.org/
> 
> Fixes: 44dc5c41b5b1 ("tracing: Fix wasted memory in saved_cmdlines logic")
> Reported-by: Kalle Valo 
> Signed-off-by: Steven Rostedt (Google) 

Reviewed-by: Catalin Marinas 



[PATCH] tracing: Inform kmemleak of saved_cmdlines allocation

2024-02-14 Thread Steven Rostedt
From: "Steven Rostedt (Google)" 

The allocation of the struct saved_cmdlines_buffer structure changed from:

s = kmalloc(sizeof(*s), GFP_KERNEL);
s->saved_cmdlines = kmalloc_array(TASK_COMM_LEN, val, GFP_KERNEL);

to:

orig_size = sizeof(*s) + val * TASK_COMM_LEN;
order = get_order(orig_size);
size = 1 << (order + PAGE_SHIFT);
page = alloc_pages(GFP_KERNEL, order);
if (!page)
return NULL;

s = page_address(page);
memset(s, 0, sizeof(*s));

s->saved_cmdlines = kmalloc_array(TASK_COMM_LEN, val, GFP_KERNEL);

Where that s->saved_cmdlines allocation looks to be a dangling allocation
to kmemleak. That's because kmemleak only keeps track of kmalloc()
allocations. For allocations that use page_alloc() directly, the kmemleak
needs to be explicitly informed about it.

Add kmemleak_alloc() and kmemleak_free() around the page allocation so
that it doesn't give the following false positive:

unreferenced object 0x8881010c8000 (size 32760):
  comm "swapper", pid 0, jiffies 4294667296
  hex dump (first 32 bytes):
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  
  backtrace (crc ae6ec1b9):
[] kmemleak_alloc+0x45/0x80
[] __kmalloc_large_node+0x10d/0x190
[] __kmalloc+0x3b1/0x4c0
[] allocate_cmdlines_buffer+0x113/0x230
[] tracer_alloc_buffers.isra.0+0x124/0x460
[] early_trace_init+0x14/0xa0
[] start_kernel+0x12e/0x3c0
[] x86_64_start_reservations+0x18/0x30
[] x86_64_start_kernel+0x7b/0x80
[] secondary_startup_64_no_verify+0x15e/0x16b

Link: https://lore.kernel.org/linux-trace-kernel/87r0hfnr9r@kernel.org/

Fixes: 44dc5c41b5b1 ("tracing: Fix wasted memory in saved_cmdlines logic")
Reported-by: Kalle Valo 
Signed-off-by: Steven Rostedt (Google) 
---
 kernel/trace/trace_sched_switch.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/trace/trace_sched_switch.c 
b/kernel/trace/trace_sched_switch.c
index e4fbcc3bede5..de4182224ea2 100644
--- a/kernel/trace/trace_sched_switch.c
+++ b/kernel/trace/trace_sched_switch.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "trace.h"
@@ -190,6 +191,7 @@ static void free_saved_cmdlines_buffer(struct 
saved_cmdlines_buffer *s)
int order = get_order(sizeof(*s) + s->cmdline_num * TASK_COMM_LEN);
 
kfree(s->map_cmdline_to_pid);
+   kmemleak_free(s);
free_pages((unsigned long)s, order);
 }
 
@@ -210,6 +212,7 @@ static struct saved_cmdlines_buffer 
*allocate_cmdlines_buffer(unsigned int val)
 
s = page_address(page);
memset(s, 0, sizeof(*s));
+   kmemleak_alloc(s, size, 1, GFP_KERNEL);
 
/* Round up to actual allocation */
val = (size - sizeof(*s)) / TASK_COMM_LEN;
-- 
2.43.0