| Message ID | 35cab46a.6d145.19334919ed0.Coremail.zhangkai@iscas.ac.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | riscv: fix memory leakage in process_accumulated_relocations | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/vmtest-fixes-PR | fail | merge-conflict |
On 16/11/2024 11:42, zhangkai@iscas.ac.cn wrote: > When module relocation is done, process_accumulated_relocations() > frees all dynamic allocated memory. rel_head_iter->rel_entry is > missed to free that kmemleak might report: > > unreferenced object 0xffffffd880c5fc40 (size 16): > comm "insmod", pid 1101, jiffies 4295045138 > hex dump (first 16 bytes): > e0 c0 f5 87 d8 ff ff ff 60 c5 f5 87 d8 ff ff ff ........`....... > backtrace (crc d2ecb20c): > [<00000000b01655f6>] kmalloc_trace_noprof+0x268/0x2f6 > [<000000006dc0067a>] add_relocation_to_accumulate.constprop.0+0xf2/0x1aa > [<00000000e1b29a36>] apply_relocate_add+0x13c/0x36e > [<000000007543f1fb>] load_module+0x5c6/0x83e > [<00000000abce12e8>] init_module_from_file+0x74/0xaa > [<0000000049413e3d>] idempotent_init_module+0x116/0x22e > [<00000000f9b98b85>] __riscv_sys_finit_module+0x62/0xae > > Signed-off-by: Kai Zhang <zhangkai@iscas.ac.cn> > --- > arch/riscv/kernel/module.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c > index 1cd461f3d87..f8c3c4b47dc 100644 > --- a/arch/riscv/kernel/module.c > +++ b/arch/riscv/kernel/module.c > @@ -643,6 +643,7 @@ process_accumulated_relocations(struct module *me, > } > reloc_handlers[curr_type].accumulate_handler( > me, location, buffer); > + kfree(rel_head_iter->rel_entry); Hey Kai, Your patch output is messed up probably due to some encoding issue. But looknig at it and the module code, it seems like rel_entry does not need to be a pointer. It can be a plain "struct list_head rel_entry" and then simply pass the list_head pointer rather than allocating/freeing it. That remove an allocation as well as some memory leak. BTW, for fixes, you should add a Fixes: tag with the original commit sha1 that introduced the bug. Thanks, Clément > kfree(rel_head_iter); > } > kfree(bucket_iter);
On Tue, 2024-11-19 at 17:57 +0100, Clément Léger wrote: > > > On 16/11/2024 11:42, zhangkai@iscas.ac.cn wrote: > > When module relocation is done, process_accumulated_relocations() > > frees all dynamic allocated memory. rel_head_iter->rel_entry is > > missed to free that kmemleak might report: > > > > unreferenced object 0xffffffd880c5fc40 (size 16): > > comm "insmod", pid 1101, jiffies 4295045138 > > hex dump (first 16 bytes): > > e0 c0 f5 87 d8 ff ff ff 60 c5 f5 87 d8 ff ff ff > > ........`....... > > backtrace (crc d2ecb20c): > > [<00000000b01655f6>] kmalloc_trace_noprof+0x268/0x2f6 > > [<000000006dc0067a>] > > add_relocation_to_accumulate.constprop.0+0xf2/0x1aa > > [<00000000e1b29a36>] apply_relocate_add+0x13c/0x36e > > [<000000007543f1fb>] load_module+0x5c6/0x83e > > [<00000000abce12e8>] init_module_from_file+0x74/0xaa > > [<0000000049413e3d>] idempotent_init_module+0x116/0x22e > > [<00000000f9b98b85>] __riscv_sys_finit_module+0x62/0xae > > > > Signed-off-by: Kai Zhang <zhangkai@iscas.ac.cn> > > --- > > arch/riscv/kernel/module.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/riscv/kernel/module.c > > b/arch/riscv/kernel/module.c > > index 1cd461f3d87..f8c3c4b47dc 100644 > > --- a/arch/riscv/kernel/module.c > > +++ b/arch/riscv/kernel/module.c > > @@ -643,6 +643,7 @@ process_accumulated_relocations(struct module > > *me, > > } > > > > reloc_handlers[curr_type].accumulate_handler( > > me, location, buffer); > > + kfree(rel_head_iter->rel_entry); > > Hey Kai, > > Your patch output is messed up probably due to some encoding issue. Sorry, I've already resent one. > But looknig at it and the module code, it seems like rel_entry does > not > need to be a pointer. It can be a plain "struct list_head rel_entry" > and I didn't find the plain(none pointer) usage. Where please? > then simply pass the list_head pointer rather than allocating/freeing > it. That remove an allocation as well as some memory leak. > > BTW, for fixes, you should add a Fixes: tag with the original commit > sha1 that introduced the bug. Thank you. I'll try. laokz > Thanks, > > Clément > > > kfree(rel_head_iter); > > } > > kfree(bucket_iter);
diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c index 1cd461f3d87..f8c3c4b47dc 100644 --- a/arch/riscv/kernel/module.c +++ b/arch/riscv/kernel/module.c @@ -643,6 +643,7 @@ process_accumulated_relocations(struct module *me, } reloc_handlers[curr_type].accumulate_handler( me, location, buffer); + kfree(rel_head_iter->rel_entry); kfree(rel_head_iter); } kfree(bucket_iter);
When module relocation is done, process_accumulated_relocations() frees all dynamic allocated memory. rel_head_iter->rel_entry is missed to free that kmemleak might report: unreferenced object 0xffffffd880c5fc40 (size 16): comm "insmod", pid 1101, jiffies 4295045138 hex dump (first 16 bytes): e0 c0 f5 87 d8 ff ff ff 60 c5 f5 87 d8 ff ff ff ........`....... backtrace (crc d2ecb20c): [<00000000b01655f6>] kmalloc_trace_noprof+0x268/0x2f6 [<000000006dc0067a>] add_relocation_to_accumulate.constprop.0+0xf2/0x1aa [<00000000e1b29a36>] apply_relocate_add+0x13c/0x36e [<000000007543f1fb>] load_module+0x5c6/0x83e [<00000000abce12e8>] init_module_from_file+0x74/0xaa [<0000000049413e3d>] idempotent_init_module+0x116/0x22e [<00000000f9b98b85>] __riscv_sys_finit_module+0x62/0xae Signed-off-by: Kai Zhang <zhangkai@iscas.ac.cn> --- arch/riscv/kernel/module.c | 1 + 1 file changed, 1 insertion(+)