On Mon, Jan 08, 2024 at 11:11:08PM +0800, Su Yue wrote: > The pointer dev_name can be modified by strseq(), > then causes the memleak: > > unreferenced object 0xffff9d08a2916c80 (size 32): > comm "mount.bcachefs", pid 9090, jiffies 4295856224 (age 17.564s) > hex dump (first 32 bytes): > 2f 64 65 76 2f 6d 61 70 70 65 72 2f 74 65 73 74 /dev/mapper/test > 2d 30 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -0.............. > backtrace: > [<00000000c5d3be7d>] __kmem_cache_alloc_node+0x1f3/0x2c0 > [<0000000052215d26>] __kmalloc_node_track_caller+0x51/0x150 > [<0000000069fea956>] kstrdup+0x32/0x60 > [<000000000877fcf1>] bch2_split_devs+0x3f/0x150 [bcachefs] > [<000000007ee93204>] bch2_mount+0xcb/0x640 [bcachefs] > [<000000002dd1e04b>] legacy_get_tree+0x30/0x60 > [<000000006afc31d3>] vfs_get_tree+0x28/0xf0 > [<000000007b0c538e>] path_mount+0x475/0xb60 > [<0000000092de5882>] __x64_sys_mount+0x105/0x140 > [<0000000054fc05d8>] do_syscall_64+0x42/0xf0 > [<00000000df584910>] entry_SYSCALL_64_after_hwframe+0x6e/0x76 > > Fix it by copy pointer dev_name at beginning and free the copied > pointer at end. > > Signed-off-by: Su Yue <[email protected]> > --- > changelog: > v3: > Add the missing the memleak report. > v2: > Remove the unnecessary assignment. > ---
LGTM, thanks for the tweak: Reviewed-by: Brian Foster <[email protected]> Now that I think about it, I suspect Kent might want to move the *s declaration to precede the while loop, but that's a separate topic. ;) Brian > fs/bcachefs/util.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/fs/bcachefs/util.c b/fs/bcachefs/util.c > index c2ef7cddaa4f..f927c8a19e24 100644 > --- a/fs/bcachefs/util.c > +++ b/fs/bcachefs/util.c > @@ -1186,7 +1186,9 @@ int bch2_split_devs(const char *_dev_name, darray_str > *ret) > { > darray_init(ret); > > - char *dev_name = kstrdup(_dev_name, GFP_KERNEL), *s = dev_name; > + char *dev_name, *s, *orig; > + > + dev_name = orig = kstrdup(_dev_name, GFP_KERNEL); > if (!dev_name) > return -ENOMEM; > > @@ -1201,10 +1203,10 @@ int bch2_split_devs(const char *_dev_name, darray_str > *ret) > } > } > > - kfree(dev_name); > + kfree(orig); > return 0; > err: > bch2_darray_str_exit(ret); > - kfree(dev_name); > + kfree(orig); > return -ENOMEM; > } > -- > 2.43.0 >
