| Message ID | 1624019875-611-1-git-send-email-faiyazm@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] mm: slub: fix the leak of alloc/free traces debugfs interface | expand |
On Fri, Jun 18, 2021 at 3:38 PM Faiyaz Mohammed <faiyazm@codeaurora.org> wrote: > > fix the leak of alloc/free traces debugfs interface, reported Fix > by kmemleak like below, > > unreferenced object 0xffff00091ae1b540 (size 64): > comm "lsbug", pid 1607, jiffies 4294958291 (age 1476.340s) > hex dump (first 32 bytes): > 02 00 00 00 00 00 00 00 6b 6b 6b 6b 6b 6b 6b 6b ........kkkkkkkk > 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk > backtrace: > [<ffff8000106b06b8>] slab_post_alloc_hook+0xa0/0x418 > [<ffff8000106b5c7c>] kmem_cache_alloc_trace+0x1e4/0x378 > [<ffff8000106b5e40>] slab_debugfs_start+0x30/0x50 > slab_debugfs_start at mm/slub.c:5831 > [<ffff8000107b3dbc>] seq_read_iter+0x214/0xd50 > [<ffff8000107b4b84>] seq_read+0x28c/0x418 > [<ffff8000109560b4>] full_proxy_read+0xdc/0x148 > [<ffff800010738f24>] vfs_read+0x104/0x340 > [<ffff800010739ee0>] ksys_read+0xf8/0x1e0 > [<ffff80001073a03c>] __arm64_sys_read+0x74/0xa8 > [<ffff8000100358d4>] invoke_syscall.constprop.0+0xdc/0x1d8 > [<ffff800010035ab4>] do_el0_svc+0xe4/0x298 > [<ffff800011138528>] el0_svc+0x20/0x30 > [<ffff800011138b08>] el0t_64_sync_handler+0xb0/0xb8 > [<ffff80001001259c>] el0t_64_sync+0x178/0x17c Can you shrink this a bit? > Fixes: 84a2bdb1b458fc968d6d9e07dab388dc679bd747 ("mm: slub: move sysfs slab alloc/free interfaces to debugfs") We use 12, which is shorter. > Link: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/mm/slub.c?h=next-20210617&id=84a2bdb1b458fc968d6d9e07dab388dc679bd747 > Must be no blank lines in the tag block. > Signed-off-by: Faiyaz Mohammed <faiyazm@codeaurora.org> ... > static void *slab_debugfs_next(struct seq_file *seq, void *v, loff_t *ppos) > { > - loff_t *spos = v; > struct loc_track *t = seq->private; > > + v = ppos; > if (*ppos < t->count) { > - *ppos = ++*spos; > - return spos; > + ++*ppos; > + return v; > } > - *ppos = ++*spos; > + ++*ppos; > return NULL; Can it be v = ppos; ++*ppos; if (*ppos <= t->count) return v; return NULL; ? (basically the question is, is the comparison equivalent in this case or not) > }
On 6/18/2021 6:45 PM, Andy Shevchenko wrote: > On Fri, Jun 18, 2021 at 3:38 PM Faiyaz Mohammed <faiyazm@codeaurora.org> wrote: >> >> fix the leak of alloc/free traces debugfs interface, reported > > Fix > Okay, I will update in next patch version. >> by kmemleak like below, >> >> unreferenced object 0xffff00091ae1b540 (size 64): >> comm "lsbug", pid 1607, jiffies 4294958291 (age 1476.340s) >> hex dump (first 32 bytes): >> 02 00 00 00 00 00 00 00 6b 6b 6b 6b 6b 6b 6b 6b ........kkkkkkkk >> 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk >> backtrace: >> [<ffff8000106b06b8>] slab_post_alloc_hook+0xa0/0x418 >> [<ffff8000106b5c7c>] kmem_cache_alloc_trace+0x1e4/0x378 >> [<ffff8000106b5e40>] slab_debugfs_start+0x30/0x50 >> slab_debugfs_start at mm/slub.c:5831 >> [<ffff8000107b3dbc>] seq_read_iter+0x214/0xd50 >> [<ffff8000107b4b84>] seq_read+0x28c/0x418 >> [<ffff8000109560b4>] full_proxy_read+0xdc/0x148 >> [<ffff800010738f24>] vfs_read+0x104/0x340 >> [<ffff800010739ee0>] ksys_read+0xf8/0x1e0 >> [<ffff80001073a03c>] __arm64_sys_read+0x74/0xa8 >> [<ffff8000100358d4>] invoke_syscall.constprop.0+0xdc/0x1d8 >> [<ffff800010035ab4>] do_el0_svc+0xe4/0x298 >> [<ffff800011138528>] el0_svc+0x20/0x30 >> [<ffff800011138b08>] el0t_64_sync_handler+0xb0/0xb8 >> [<ffff80001001259c>] el0t_64_sync+0x178/0x17c > > Can you shrink this a bit? > Okay >> Fixes: 84a2bdb1b458fc968d6d9e07dab388dc679bd747 ("mm: slub: move sysfs slab alloc/free interfaces to debugfs") > > We use 12, which is shorter. > >> Link: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/mm/slub.c?h=next-20210617&id=84a2bdb1b458fc968d6d9e07dab388dc679bd747 > >> > > Must be no blank lines in the tag block. > >> Signed-off-by: Faiyaz Mohammed <faiyazm@codeaurora.org> > Okay > ... > >> static void *slab_debugfs_next(struct seq_file *seq, void *v, loff_t *ppos) >> { >> - loff_t *spos = v; >> struct loc_track *t = seq->private; >> >> + v = ppos; >> if (*ppos < t->count) { >> - *ppos = ++*spos; >> - return spos; >> + ++*ppos; >> + return v; >> } >> - *ppos = ++*spos; >> + ++*ppos; >> return NULL; > > Can it be > > v = ppos; > ++*ppos; > if (*ppos <= t->count> return v; > return NULL; > > ? (basically the question is, is the comparison equivalent in this case or not) > >> } >Yes, we can update it and slab_debugfs_show has the index check as well. I will update in next patch version. Thanks and regards, Mohammed Faiyaz
diff --git a/mm/slub.c b/mm/slub.c index fcb0c50..f006766 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -5785,31 +5785,24 @@ static int slab_debugfs_show(struct seq_file *seq, void *v) static void slab_debugfs_stop(struct seq_file *seq, void *v) { - kfree(v); } static void *slab_debugfs_next(struct seq_file *seq, void *v, loff_t *ppos) { - loff_t *spos = v; struct loc_track *t = seq->private; + v = ppos; if (*ppos < t->count) { - *ppos = ++*spos; - return spos; + ++*ppos; + return v; } - *ppos = ++*spos; + ++*ppos; return NULL; } static void *slab_debugfs_start(struct seq_file *seq, loff_t *ppos) { - loff_t *spos = kmalloc(sizeof(loff_t), GFP_KERNEL); - - if (!spos) - return NULL; - - *spos = *ppos; - return spos; + return ppos; } static const struct seq_operations slab_debugfs_sops = {
fix the leak of alloc/free traces debugfs interface, reported by kmemleak like below, unreferenced object 0xffff00091ae1b540 (size 64): comm "lsbug", pid 1607, jiffies 4294958291 (age 1476.340s) hex dump (first 32 bytes): 02 00 00 00 00 00 00 00 6b 6b 6b 6b 6b 6b 6b 6b ........kkkkkkkk 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk backtrace: [<ffff8000106b06b8>] slab_post_alloc_hook+0xa0/0x418 [<ffff8000106b5c7c>] kmem_cache_alloc_trace+0x1e4/0x378 [<ffff8000106b5e40>] slab_debugfs_start+0x30/0x50 slab_debugfs_start at mm/slub.c:5831 [<ffff8000107b3dbc>] seq_read_iter+0x214/0xd50 [<ffff8000107b4b84>] seq_read+0x28c/0x418 [<ffff8000109560b4>] full_proxy_read+0xdc/0x148 [<ffff800010738f24>] vfs_read+0x104/0x340 [<ffff800010739ee0>] ksys_read+0xf8/0x1e0 [<ffff80001073a03c>] __arm64_sys_read+0x74/0xa8 [<ffff8000100358d4>] invoke_syscall.constprop.0+0xdc/0x1d8 [<ffff800010035ab4>] do_el0_svc+0xe4/0x298 [<ffff800011138528>] el0_svc+0x20/0x30 [<ffff800011138b08>] el0t_64_sync_handler+0xb0/0xb8 [<ffff80001001259c>] el0t_64_sync+0x178/0x17c Fixes: 84a2bdb1b458fc968d6d9e07dab388dc679bd747 ("mm: slub: move sysfs slab alloc/free interfaces to debugfs") Link: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/mm/slub.c?h=next-20210617&id=84a2bdb1b458fc968d6d9e07dab388dc679bd747 Signed-off-by: Faiyaz Mohammed <faiyazm@codeaurora.org> --- mm/slub.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-)