| Message ID | 20210727025232.663-2-yuzenghui@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] bcma: Fix memory leak for internally-handled cores | expand |
On 2021/7/27 10:52, Zenghui Yu wrote: > kmemleak reported that dev_name() of internally-handled cores were leaked > on driver unbinding. Let's use device_initialize() to take refcounts for > them and put_device() to properly free the related stuff. Could this be picked as a fix for v5.14 (_if_ it does fix something)? Thanks, Zenghui
Zenghui Yu <yuzenghui@huawei.com> writes: > On 2021/7/27 10:52, Zenghui Yu wrote: >> kmemleak reported that dev_name() of internally-handled cores were leaked >> on driver unbinding. Let's use device_initialize() to take refcounts for >> them and put_device() to properly free the related stuff. > > Could this be picked as a fix for v5.14 (_if_ it does fix something)? Why should this go to v5.14? Most probably it's too late for v5.14 anyway. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 2021/8/21 18:05, Kalle Valo wrote: > Zenghui Yu <yuzenghui@huawei.com> writes: > >> On 2021/7/27 10:52, Zenghui Yu wrote: >>> kmemleak reported that dev_name() of internally-handled cores were leaked >>> on driver unbinding. Let's use device_initialize() to take refcounts for >>> them and put_device() to properly free the related stuff. >> >> Could this be picked as a fix for v5.14 (_if_ it does fix something)? > > Why should this go to v5.14? Most probably it's too late for v5.14 > anyway. No worries. It's not urgent and just that I didn't realize we're already at rc7 now. The patch has been on the list for about 4 weeks and I'd appreciate any review comments from maintainers. Zenghui
Zenghui Yu <yuzenghui@huawei.com> wrote: > kmemleak reported that dev_name() of internally-handled cores were leaked > on driver unbinding. Let's use device_initialize() to take refcounts for > them and put_device() to properly free the related stuff. > > While looking at it, there's another potential issue for those which should > be *registered* into driver core. If device_register() failed, we put > device once and freed bcma_device structures. In bcma_unregister_cores(), > they're treated as unregistered and we hit both UAF and double-free. That > smells not good and has also been fixed now. > > Fixes: ab54bc8460b5 ("bcma: fill core details for every device") > Signed-off-by: Zenghui Yu <yuzenghui@huawei.com> 2 patches applied to wireless-drivers-next.git, thanks. b63aed3ff195 bcma: Fix memory leak for internally-handled cores 9fc8048c56f3 bcma: Drop the unused parameter of bcma_scan_read32() -- https://patchwork.kernel.org/project/linux-wireless/patch/20210727025232.663-2-yuzenghui@huawei.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c index 6535614a7dc1..1df2b5801c3b 100644 --- a/drivers/bcma/main.c +++ b/drivers/bcma/main.c @@ -236,6 +236,7 @@ EXPORT_SYMBOL(bcma_core_irq); void bcma_prepare_core(struct bcma_bus *bus, struct bcma_device *core) { + device_initialize(&core->dev); core->dev.release = bcma_release_core_dev; core->dev.bus = &bcma_bus_type; dev_set_name(&core->dev, "bcma%d:%d", bus->num, core->core_index); @@ -277,11 +278,10 @@ static void bcma_register_core(struct bcma_bus *bus, struct bcma_device *core) { int err; - err = device_register(&core->dev); + err = device_add(&core->dev); if (err) { bcma_err(bus, "Could not register dev for core 0x%03X\n", core->id.id); - put_device(&core->dev); return; } core->dev_registered = true; @@ -372,7 +372,7 @@ void bcma_unregister_cores(struct bcma_bus *bus) /* Now noone uses internally-handled cores, we can free them */ list_for_each_entry_safe(core, tmp, &bus->cores, list) { list_del(&core->list); - kfree(core); + put_device(&core->dev); } }
kmemleak reported that dev_name() of internally-handled cores were leaked on driver unbinding. Let's use device_initialize() to take refcounts for them and put_device() to properly free the related stuff. While looking at it, there's another potential issue for those which should be *registered* into driver core. If device_register() failed, we put device once and freed bcma_device structures. In bcma_unregister_cores(), they're treated as unregistered and we hit both UAF and double-free. That smells not good and has also been fixed now. Fixes: ab54bc8460b5 ("bcma: fill core details for every device") Signed-off-by: Zenghui Yu <yuzenghui@huawei.com> --- drivers/bcma/main.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)