Re: [PATCH RFC v8 8/8] mfd: ls2kbmc: Able to be compiled as a module

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

Pre-existing issues:
- [Critical] System panic/freeze due to >10 seconds delay inside `stop_machine` context.
- [High] Dangling pointer in global state and MFD guideline violation due to dynamic modification of `mfd_cell` array.
--

--- Patch [8]: [PATCH RFC v8 8/8] mfd: ls2kbmc: Able to be compiled as a module ---
commit 3dfba1198ec7bd502a9fc1a937060e6136a907cd
Author: Miao Wang <[email protected]>

mfd: ls2kbmc: Able to be compiled as a module
    
Make ls2kbmc able to be compiled as a module.

> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 763ce6a34782b..a7a9f97af248c 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2494,7 +2494,7 @@ config MFD_LOONGSON_SE
>  	  which will forward them to the corresponding engine.
>  
>  config MFD_LS2K_BMC_CORE
> -	bool "Loongson-2K Board Management Controller Support"
> +	tristate "Loongson-2K Board Management Controller Support"

[Severity: Critical]
This is a pre-existing issue, but will using stop_machine() with long delays
cause system freezes?

When the driver is built as a module, or even built-in, ls2k_bmc_events_fn()
invokes stop_machine():

drivers/mfd/ls2k-bmc-core.c:ls2k_bmc_events_fn() {
    ...
	stop_machine(ls2k_bmc_recover_pci_data, ddata, NULL);
    ...
}

The callback ls2k_bmc_recover_pci_data() executes a loop with mdelay(1) up to
2000 times, and later calls mdelay(LS2K_BMC_RESET_WAIT_TIME) which is
10000 ms:

drivers/mfd/ls2k-bmc-core.c:ls2k_bmc_recover_pci_data() {
    ...
	for (i = LS7A_BAR0_CHECK_MAX_TIMES; i > 0 ; i--) {
		if (!ls2k_bmc_bar0_addr_is_set(parent))
			break;
		mdelay(1);
	}
    ...
	/* Waiting for U-Boot and DDR ready */
	mdelay(LS2K_BMC_RESET_WAIT_TIME);
    ...
}

stop_machine() disables local interrupts and halts all online CPUs to run the
provided callback. Busy-waiting for over 10 seconds with interrupts globally
disabled could consistently trigger a kernel panic via the NMI watchdog or
hard lockup detector, causing severe RCU stalls and dropped hardware events.

Is it possible to handle this PCI recovery without halting the entire machine
for 10 seconds?

[Severity: High]
This is a pre-existing issue, but could this code leave a dangling pointer in
the global MFD cell array?

Now that the driver can be compiled as a module, it can be unloaded and
reloaded, making this more likely to cause issues.

drivers/mfd/ls2k-bmc-core.c:
static struct mfd_cell ls2k_bmc_cells[] = {
    ...
};

In ls2k_bmc_probe(), the driver dynamically mutates this global array to
store a pointer to a local stack variable pd:

drivers/mfd/ls2k-bmc-core.c:ls2k_bmc_probe() {
	struct simplefb_platform_data pd;
    ...
	ls2k_bmc_cells[LS2K_BMC_DISPLAY].platform_data = &pd;
	ls2k_bmc_cells[LS2K_BMC_DISPLAY].pdata_size = sizeof(pd);
    ...
}

After ls2k_bmc_probe() returns, the global array retains a dangling pointer
to a freed stack frame. If the driver is unbound and bound again, or if
multiple instances exist, subsequent probes might race or potentially reuse
this stale pointer before it gets overwritten, leading to a stack
use-after-free vulnerability.

Additionally, MFD subsystem guidelines require cell arrays to be static const.

Can this be refactored to avoid mutating the global mfd_cell array with stack
addresses?

>  	depends on PCI && ACPI_GENERIC_GSI
>  	select MFD_CORE
>  	help

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8