Re: [PATCH v4 4/5] bcma: add host driver for the BCM6362 integrated WLAN backplane

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] bcma: unconditional driver unregister in bcma_modexit() after init failure
- [Medium] bcma: memory and iomap leak in bcma_host_soc_bcm6362_probe() on scan error
--

commit 400b457ad79cb1893252ac704192c2032be3a329
Author: Alessio Ferri <alessio.ferri@mythread.it>

bcma: add host driver for the BCM6362 integrated WLAN backplane

This commit adds a host driver for the BCM6362 integrated WLAN bcma
backplane, handling clock enablement, resets, big-endian I/O ops, and
emulation of per-core control registers for wrapperless cores.

> diff --git a/drivers/bcma/host_soc_bcm6362.c b/drivers/bcma/host_soc_bcm6362.c
> new file mode 100644
> index 0000000000000..e8400cb624484
> --- /dev/null
> +++ b/drivers/bcma/host_soc_bcm6362.c
[ ... ]
> +static int bcma_host_soc_bcm6362_probe(struct platform_device *pdev)
> +{
[ ... ]
> +	err = bcma_bus_register(bus);
> +	if (err) {
> +		bcma_host_soc_bcm6362_power_off(host);
> +		return err;
> +	}

[Severity: Medium]
As reported by Sashiko on v2 [1], does bcma_host_soc_bcm6362_probe() leak
core structures and their ioremap() mappings if bcma_bus_register() fails?

When bcma_bus_register() calls bcma_bus_scan(), each discovered core is
allocated, mapped via ioremap() into core->io_addr, and appended to
bus->cores. If scanning a subsequent core fails (for example, returning
-ENOMEM or -EILSEQ in bcma_bus_scan()), bcma_bus_scan() returns the error
with any previously scanned cores still on bus->cores:

drivers/bcma/scan.c:bcma_bus_scan() {
    ...
		err = bcma_get_next_core(bus, &eromptr, NULL, core_num, core);
		if (err < 0) {
			kfree(core);
			...
			goto out;
		}
    ...
}

Because bcma_host_soc_bcm6362_probe() powers off and returns without
calling bcma_unregister_cores(bus), wouldn't those allocated cores and
their core->io_addr mappings be leaked?

> diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c
> index 72f045e6ed513..4ccc9fb98f616 100644
> --- a/drivers/bcma/main.c
> +++ b/drivers/bcma/main.c
> @@ -682,6 +682,11 @@ static int __init bcma_modinit(void)
>  		pr_err("SoC host initialization failed\n");
>  		err = 0;
>  	}
> +	err = bcma_host_soc_bcm6362_register_driver();
> +	if (err) {
> +		pr_err("BCM6362 WLAN host initialization failed\n");
> +		err = 0;
> +	}
>  #ifdef CONFIG_BCMA_HOST_PCI
>  	err = bcma_host_pci_init();
>  	if (err) {
> @@ -699,6 +704,7 @@ static void __exit bcma_modexit(void)
>  #ifdef CONFIG_BCMA_HOST_PCI
>  	bcma_host_pci_exit();
>  #endif
> +	bcma_host_soc_bcm6362_unregister_driver();

[Severity: Medium]
As reported by Sashiko on v2 [1], could calling
bcma_host_soc_bcm6362_unregister_driver() unconditionally in bcma_modexit()
trigger a warning if registration failed during module init?

In bcma_modinit(), if bcma_host_soc_bcm6362_register_driver() fails, the
error is logged and cleared to 0 so the module still loads, leaving
bcma_host_soc_bcm6362_driver.driver.p as NULL. When the module is later
unloaded, bcma_modexit() calls:

bcma_host_soc_bcm6362_unregister_driver()
  -> platform_driver_unregister()
    -> driver_unregister()

which checks if (!drv || !drv->p) and hits WARN(1, "Unexpected driver
unregister!\n").

[1] https://lore.kernel.org/r/20261012-bcm6362-wlan-v2-4-4752da7e0982@mythread.it

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-bcm6362-wlan-v4-0-f001f6d75dc3@mythread.it?part=4