two small bcma patches

two small bcma patches

[Zenghui Yu]

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 1/2] bcma: Fix memory leak for internally-handled cores

[Zenghui Yu]

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

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

[Zenghui Yu]

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

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

[Zenghui Yu]

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

[Kalle Valo]

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

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

[Zenghui Yu]

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

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

[Kalle Valo]

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