Re: [PATCH v2 3/6] firmware: qcom: scm: Add support for setting Bluetooth power modes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Command ID collision causing false positive availability check and potential TrustZone memory corruption.
--

commit ad188a38cad8a7f123b53d24cb2303340d519071
Author: George Moussalem <george.moussalem@outlook.com>

firmware: qcom: scm: Add support for setting Bluetooth power modes

This commit adds support for configuring the Bluetooth subsystem power
optimization mode on the IPQ5018 SoC. It introduces a new SCM call to
TrustZone and checks for the call's availability before invoking it.

> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c

[ ... ]

> +static int __qcom_scm_pas_set_bluetooth_power_mode(u32 pas_id, u32 val)
> +{
> +	struct qcom_scm_desc desc = {
> +		.svc = QCOM_SCM_SVC_PIL,
> +		.cmd = QCOM_SCM_PIL_PAS_BT_PWR_MODE,
> +		.arginfo = QCOM_SCM_ARGS(2),

[ ... ]

> +int qcom_scm_pas_set_bluetooth_power_mode(u32 pas_id, u32 val)
> +{
> +	if (!__qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_PIL,
> +					  QCOM_SCM_PIL_PAS_BT_PWR_MODE))
> +		return -EOPNOTSUPP;

[ ... ]

> diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h
> --- a/drivers/firmware/qcom/qcom_scm.h
> +++ b/drivers/firmware/qcom/qcom_scm.h
> @@ -105,6 +105,7 @@ int qcom_scm_shm_bridge_enable(struct device *scm_dev);
>  #define QCOM_SCM_PIL_PAS_SHUTDOWN	0x06
>  #define QCOM_SCM_PIL_PAS_IS_SUPPORTED	0x07
>  #define QCOM_SCM_PIL_PAS_MSS_RESET	0x0a
> +#define QCOM_SCM_PIL_PAS_BT_PWR_MODE	0x21
>  #define QCOM_SCM_PIL_PAS_GET_RSCTABLE	0x21

[Severity: High]
Could this command ID overlap cause a false positive in the availability check
and trigger unintended behavior in TrustZone?

Because QCOM_SCM_PIL_PAS_BT_PWR_MODE and QCOM_SCM_PIL_PAS_GET_RSCTABLE are
both defined as 0x21 under the same service (QCOM_SCM_SVC_PIL), the call
to __qcom_scm_is_call_available() in qcom_scm_pas_set_bluetooth_power_mode()
will return true on any platform that supports getting the resource table,
even if it does not actually support the Bluetooth power mode command.

If the function proceeds, it will invoke command 0x21 with only 2 arguments
instead of the 5 expected by the GET_RSCTABLE handler. Since the unprovided
arguments default to 0, might this cause TrustZone to read or write at
physical address 0x0, potentially resulting in a Secure World crash or
memory corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-ipq5018-bluetooth-v2-0-02770f03b6bb@outlook.com?part=3