Hi all,
two small bcma patches -- one to fix memory leakage for internally-handled
cores, and the other one to remove the never used parameter.
--
2.19.1
As it had never been used since the initial commit 8369ae33b705 ("bcma: add
Broadcom specific AMBA bus driver").
Signed-off-by: Zenghui Yu <[email protected]>
---
drivers/bcma/scan.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/bcma/scan.c b/drivers/bcma/scan.c
index d49e7c0de2b6..26d12a7e6ca0 100644
--- a/drivers/bcma/scan.c
+++ b/drivers/bcma/scan.c
@@ -141,8 +141,7 @@ static const char *bcma_device_name(const struct bcma_device_id *id)
return "UNKNOWN";
}
-static u32 bcma_scan_read32(struct bcma_bus *bus, u8 current_coreidx,
- u16 offset)
+static u32 bcma_scan_read32(struct bcma_bus *bus, u16 offset)
{
return readl(bus->mmio + offset);
}
@@ -443,7 +442,7 @@ void bcma_detect_chip(struct bcma_bus *bus)
bcma_scan_switch_core(bus, BCMA_ADDR_BASE);
- tmp = bcma_scan_read32(bus, 0, BCMA_CC_ID);
+ tmp = bcma_scan_read32(bus, BCMA_CC_ID);
chipinfo->id = (tmp & BCMA_CC_ID_ID) >> BCMA_CC_ID_ID_SHIFT;
chipinfo->rev = (tmp & BCMA_CC_ID_REV) >> BCMA_CC_ID_REV_SHIFT;
chipinfo->pkg = (tmp & BCMA_CC_ID_PKG) >> BCMA_CC_ID_PKG_SHIFT;
@@ -465,7 +464,7 @@ int bcma_bus_scan(struct bcma_bus *bus)
if (bus->nr_cores)
return 0;
- erombase = bcma_scan_read32(bus, 0, BCMA_CC_EROM);
+ erombase = bcma_scan_read32(bus, BCMA_CC_EROM);
if (bus->hosttype == BCMA_HOSTTYPE_SOC) {
eromptr = ioremap(erombase, BCMA_CORE_SIZE);
if (!eromptr)
--
2.19.1
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 <[email protected]>
---
drivers/bcma/main.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
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);
}
}
--
2.19.1
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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]>
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/[email protected]/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches