Re: [PATCH v5 13/16] power: sequencing: pcie-m2: Add usb and sdio targets for E-key connector

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing initialization of `list_head` causes a NULL pointer dereference on the `pwrseq_unit_setup()` error path.
--

--- Patch [13]: [PATCH v5 13/16] power: sequencing: pcie-m2: Add usb and sdio targets for E-key connector ---
commit a4efebd06d30c32580111345a5d6a16748b52be2
Author: Chen-Yu Tsai <wenst@chromium.org>

power: sequencing: pcie-m2: Add usb and sdio targets for E-key connector

The M.2 E-key connector allows either PCIe or SDIO for WiFi and USB or
UART for BT. This commit adds power sequencing targets for SDIO and USB
by renaming existing units to wifi and bt.

> diff --git a/drivers/power/sequencing/pwrseq-pcie-m2.c b/drivers/power/sequencing/pwrseq-pcie-m2.c
> index 01f9f6a717464..8ae55e87db74c 100644
> --- a/drivers/power/sequencing/pwrseq-pcie-m2.c
> +++ b/drivers/power/sequencing/pwrseq-pcie-m2.c
> @@ -69,46 +69,46 @@ static const struct pwrseq_unit_data *pwrseq_pcie_m2_unit_deps[] = {
[ ... ]
> -static const struct pwrseq_unit_data pwrseq_pcie_m2_e_uart_unit_data = {
> -	.name = "uart-enable",
> +static const struct pwrseq_unit_data pwrseq_pcie_m2_e_bt_unit_data = {
> +	.name = "bt-enable",
>  	.deps = pwrseq_pcie_m2_unit_deps,
> -	.enable = pwrseq_pci_m2_e_uart_enable,
> -	.disable = pwrseq_pci_m2_e_uart_disable,
> +	.enable = pwrseq_pci_m2_e_bt_enable,
> +	.disable = pwrseq_pci_m2_e_bt_disable,
>  };

[Severity: High]
This is a pre-existing issue, but while reviewing the new power sequencing
units being added here, I noticed a potential NULL pointer dereference in
the core power sequencing error path.

Does the error path in pwrseq_unit_setup() crash when freeing an uninitialized
list?

When a unit is allocated in drivers/power/sequencing/core.c:pwrseq_unit_new(),
the unit->list field is not initialized:

drivers/power/sequencing/core.c:pwrseq_unit_new() {
	...
	kref_init(&unit->ref);
	INIT_LIST_HEAD(&unit->deps);
	unit->enable = data->enable;
	...
}

If pwrseq_unit_setup() fails before adding the unit to the list (for example,
due to an out-of-memory error), the cleanup path calls pwrseq_unit_put(unit).
This triggers pwrseq_unit_release():

drivers/power/sequencing/core.c:pwrseq_unit_release() {
	...
	pwrseq_unit_free_deps(&unit->deps);
	list_del(&unit->list);
	kfree_const(unit->name);
	...
}

Since INIT_LIST_HEAD(&unit->list) was never called, the list pointers are
still NULL. Calling list_del(&unit->list) will attempt to modify next->prev,
resulting in an immediate kernel panic.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715085348.3457359-1-wenst@chromium.org?part=13