two small bcma patches

5 messages, 2 authors, 2021-08-29 · open the first message on its own page

two small bcma patches

From: Zenghui Yu <yuzenghui@huawei.com>
Date: 2021-07-27 02:52:54

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

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

From: Zenghui Yu <yuzenghui@huawei.com>
Date: 2021-07-27 02:52:59

As it had never been used since the initial commit 8369ae33b705 ("bcma: add
Broadcom specific AMBA bus driver").

Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
---
 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

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

From: Zenghui Yu <yuzenghui@huawei.com>
Date: 2021-07-27 02:53:02

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(-)
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

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

From: Zenghui Yu <yuzenghui@huawei.com>
Date: 2021-08-21 03:29:02

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

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

From: Kalle Valo <hidden>
Date: 2021-08-29 11:45:32

Zenghui Yu [off-list ref] 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help