2021-07-27 02:53:15

by Zenghui Yu

[permalink] [raw]
Subject: two small bcma patches

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


2021-07-27 02:53:17

by Zenghui Yu

[permalink] [raw]
Subject: [PATCH 2/2] bcma: Drop the unused parameter of bcma_scan_read32()

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

2021-07-27 02:54:20

by Zenghui Yu

[permalink] [raw]
Subject: [PATCH 1/2] bcma: Fix memory leak for internally-handled cores

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