Re: [PATCH 2/4] bcma: allow SHIM-style mini-EROM wrapper-less cores in scan

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Unconditional dereference of `cc->core` in dependent modules can cause a kernel Oops if core scanning fails or a core is missing.
- [High] The fallback bridge check in `bcma_get_next_core()` fails to skip the rest of the component's EROM descriptors before returning `-ENXIO`.
--

commit 257794fc3cb2f2e7a20011d91e6239f7a6956709
Author: Alessio Ferri <alessio.ferri@mythread.it>

bcma: allow SHIM-style mini-EROM wrapper-less cores in scan

This commit modifies bcma_get_next_core() to allow certain wrapper-less
components on systems with a specific mini-erom layout. It resolves a
silent failure where skipped components lead to an empty core list,
eventually causing a null pointer dereference during probe.

> On SoCs that publish a SHIM-style mini-EROM (i.e. BCM6362) the
> WLAN backplane lists three components:
> ChipCommon, IEEE 802.11 and BCMA_CORE_SHIM. None of the three is
> in the existing allowlist, so all three are skipped silently,
> bus->cores stays empty, bcma_find_core(BCMA_CORE_CHIPCOMMON)
> returns NULL, and a later bcma_chipco_watchdog_register()
> dereferences cc->core->bus on its first line and oopses mid-probe.

[Severity: High]
This is a pre-existing issue, but does fixing this specific edge case
leave an underlying structural fragility unhandled?

If a hardware variant genuinely lacks a ChipCommon core or its erom is
malformed, bcma_find_core() returns null and bus->drv_cc.core remains null.

Subsequently, bcma_register_devices() calls bcma_gpio_init() and
bcma_chipco_watchdog_register(). Both of these helper functions
unconditionally dereference cc->core->bus on their first line without
checking if cc->core is valid.

This would trigger a null pointer dereference and a kernel oops mid-probe.
Since the bus can be exposed over hotpluggable pci interfaces, could a
malformed or malicious untrusted device intentionally trigger this local
denial of service?

> diff --git a/drivers/bcma/scan.c b/drivers/bcma/scan.c
> index 983a62ddeebb0..782fc53eb6b3c 100644
> --- a/drivers/bcma/scan.c
> +++ b/drivers/bcma/scan.c
> @@ -318,6 +318,21 @@ static int bcma_get_next_core(struct bcma_bus *bus, u32 __iomem **eromptr,
>  		case BCMA_CORE_GCI:
>  		/* Not used yet: case BCMA_CORE_OOB_ROUTER: */
>  			break;
> +		case BCMA_CORE_CHIPCOMMON:
> +		case BCMA_CORE_80211:
> +		case BCMA_CORE_SHIM:

[Severity: High]
This isn't a bug introduced by this patch, but does the fallback bridge
check later in bcma_get_next_core() fail to skip the rest of the component's
descriptors before returning -ENXIO?

If a bridge component has master ports, the early bcma_erom_is_bridge()
check returns false. Later, when bcma_erom_get_addr_desc() identifies a
SCAN_ADDR_TYPE_BRIDGE descriptor, it logs "Bridge found" and returns -ENXIO:

drivers/bcma/scan.c:bcma_get_next_core() {
    ...
		/* Try again to see if it is a bridge */
		tmp = bcma_erom_get_addr_desc(bus, eromptr,
					      SCAN_ADDR_TYPE_BRIDGE, 0);
		if (tmp == 0 || IS_ERR_VALUE_U32(tmp)) {
			return -EILSEQ;
		} else {
			bcma_info(bus, "Bridge found\n");
			return -ENXIO;
		}
    ...
}

Crucially, unlike other -ENXIO error paths, it omits calling
bcma_erom_skip_component(bus, eromptr).

If the bridge component has remaining descriptors like additional slave ports
or wrappers, eromptr is left misaligned mid-component. On the subsequent
iteration of the scan loop in bcma_bus_scan(), the parser would expect a
component identifier but encounter a leftover wrapper or port descriptor
instead.

Could this result in an -EILSEQ error that prematurely aborts the entire
bus initialization?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529-add-bcm6362-wlan-v1-0-722242777f58@gmail.com?part=2