[PATCH] platform: arm64: qcom-hamoa-ec: reject incomplete responses

[PATCH] platform: arm64: qcom-hamoa-ec: reject incomplete responses

[Linmao Li]

qcom_ec_read() accepts short positive transfers, while both callers
unconditionally consume every field in their fixed-size response. A short
transfer can therefore make them use trailing stack bytes that were not
returned by the device.

The first response byte contains the number of payload bytes, excluding
the byte count itself. A complete response of resp_len bytes must
therefore report resp_len - 1 payload bytes. The existing check only
rejects counts that do not fit in the response buffer and still accepts
an incomplete payload.

Require both the SMBus transfer length and the EC-provided payload count
to match the expected response size.

Fixes: 5c44f48e91de ("platform: arm64: Add driver for EC found on Qualcomm reference devices")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
The resp_len - 1 requirement is based solely on the response layouts
documented in this driver. The EC specification and hardware were not
available, so please confirm it for supported firmware revisions.

Compile-tested with CONFIG_EC_QCOM_HAMOA=m.

 drivers/platform/arm64/qcom-hamoa-ec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/arm64/qcom-hamoa-ec.c b/drivers/platform/arm64/qcom-hamoa-ec.c
index 5ca7308c6077..4d2ad042a7f8 100644
--- a/drivers/platform/arm64/qcom-hamoa-ec.c
+++ b/drivers/platform/arm64/qcom-hamoa-ec.c
@@ -92,8 +92,10 @@ static int qcom_ec_read(struct qcom_ec *ec, u8 cmd, u8 resp_len, u8 *resp)
 		return ret;
 	else if (ret == 0 || ret == 0xff)
 		return -EOPNOTSUPP;
+	else if (ret != resp_len)
+		return -EIO;
 
-	if (resp[0] >= resp_len)
+	if (resp[0] != resp_len - 1)
 		return -EINVAL;
 
 	return 0;

base-commit: c5e32e86ca02b003f86e095d379b38148999293d
-- 
2.25.1

Re: [PATCH] platform: arm64: qcom-hamoa-ec: reject incomplete responses

[Bryan O'Donoghue]

On 28/07/2026 12:19, Linmao Li wrote:
> qcom_ec_read() accepts short positive transfers, while both callers
> unconditionally consume every field in their fixed-size response. A short
> transfer can therefore make them use trailing stack bytes that were not
> returned by the device.
> 
> The first response byte contains the number of payload bytes, excluding
> the byte count itself. A complete response of resp_len bytes must
> therefore report resp_len - 1 payload bytes. The existing check only
> rejects counts that do not fit in the response buffer and still accepts
> an incomplete payload.
> 
> Require both the SMBus transfer length and the EC-provided payload count
> to match the expected response size.
> 
> Fixes: 5c44f48e91de ("platform: arm64: Add driver for EC found on Qualcomm reference devices")
> Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
> ---
> The resp_len - 1 requirement is based solely on the response layouts
> documented in this driver. The EC specification and hardware were not
> available, so please confirm it for supported firmware revisions.
> 
> Compile-tested with CONFIG_EC_QCOM_HAMOA=m.
> 
>   drivers/platform/arm64/qcom-hamoa-ec.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/arm64/qcom-hamoa-ec.c b/drivers/platform/arm64/qcom-hamoa-ec.c
> index 5ca7308c6077..4d2ad042a7f8 100644
> --- a/drivers/platform/arm64/qcom-hamoa-ec.c
> +++ b/drivers/platform/arm64/qcom-hamoa-ec.c
> @@ -92,8 +92,10 @@ static int qcom_ec_read(struct qcom_ec *ec, u8 cmd, u8 resp_len, u8 *resp)
>   		return ret;
>   	else if (ret == 0 || ret == 0xff)
>   		return -EOPNOTSUPP;
> +	else if (ret != resp_len)
> +		return -EIO;
>   
> -	if (resp[0] >= resp_len)
> +	if (resp[0] != resp_len - 1)
>   		return -EINVAL;
>   
>   	return 0;
> 
> base-commit: c5e32e86ca02b003f86e095d379b38148999293d

The logic stacks-up, interesting to see if the hardware produces blue 
smoke though.

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Re: [PATCH] platform: arm64: qcom-hamoa-ec: reject incomplete responses

[Anvesh Jain P]

On 7/28/2026 4:49 PM, Linmao Li wrote:
> qcom_ec_read() accepts short positive transfers, while both callers
> unconditionally consume every field in their fixed-size response. A short
> transfer can therefore make them use trailing stack bytes that were not
> returned by the device.
> 
> The first response byte contains the number of payload bytes, excluding
> the byte count itself. A complete response of resp_len bytes must
> therefore report resp_len - 1 payload bytes. The existing check only
> rejects counts that do not fit in the response buffer and still accepts
> an incomplete payload.
> 
> Require both the SMBus transfer length and the EC-provided payload count
> to match the expected response size.
> 
> Fixes: 5c44f48e91de ("platform: arm64: Add driver for EC found on Qualcomm reference devices")
> Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
> ---
> The resp_len - 1 requirement is based solely on the response layouts
> documented in this driver. The EC specification and hardware were not
> available, so please confirm it for supported firmware revisions.
> 
> Compile-tested with CONFIG_EC_QCOM_HAMOA=m.
> 
>  drivers/platform/arm64/qcom-hamoa-ec.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/arm64/qcom-hamoa-ec.c b/drivers/platform/arm64/qcom-hamoa-ec.c
> index 5ca7308c6077..4d2ad042a7f8 100644
> --- a/drivers/platform/arm64/qcom-hamoa-ec.c
> +++ b/drivers/platform/arm64/qcom-hamoa-ec.c
> @@ -92,8 +92,10 @@ static int qcom_ec_read(struct qcom_ec *ec, u8 cmd, u8 resp_len, u8 *resp)
>  		return ret;
>  	else if (ret == 0 || ret == 0xff)
>  		return -EOPNOTSUPP;
> +	else if (ret != resp_len)
> +		return -EIO;
>  
> -	if (resp[0] >= resp_len)
> +	if (resp[0] != resp_len - 1)
>  		return -EINVAL;
>  
>  	return 0;

Thanks for catching this — nice find.

This is independently fixed in an in-flight series that reworks
qcom_ec_read() to use raw i2c_transfer() instead of
i2c_smbus_read_i2c_block_data(), removing the partial-read case
entirely, and it already has the same resp[0] != resp_len - 1 check.
That series just hasn't landed yet, so your report stands on its own —
happy to add a Reported-by: Linmao Li <lilinmao@kylinos.cn> tag on that
commit when it's re-posted, if that works for you.

Series:
https://lore.kernel.org/r/20260728-ec_add_more_commands-v1-0-771abd65ee1a@oss.qualcomm.com

> 
> base-commit: c5e32e86ca02b003f86e095d379b38148999293d

-- 
Best Regards,
Anvesh

Re: [PATCH] platform: arm64: qcom-hamoa-ec: reject incomplete responses

[Bryan O'Donoghue]

On 29/07/2026 11:08, Anvesh Jain P wrote:
>> -	if (resp[0] >= resp_len)
>> +	if (resp[0] != resp_len - 1)
>>   		return -EINVAL;
>>   
>>   	return 0;
> Thanks for catching this — nice find.
> 
> This is independently fixed in an in-flight series that reworks
> qcom_ec_read() to use raw i2c_transfer() instead of
> i2c_smbus_read_i2c_block_data(), removing the partial-read case
> entirely, and it already has the same resp[0] != resp_len - 1 check.
> That series just hasn't landed yet, so your report stands on its own —
> happy to add a Reported-by: Linmao Li<lilinmao@kylinos.cn> tag on that
> commit when it's re-posted, if that works for you.

What about -stable ?

Seems like we should take this fix and your series should rebase on top 
of the fix instead of the corollary.

That's the way this should work Fixes > New stuff.

Can you add your Review-by/Tested-by and then rebase yourself ?

---
bod

Re: [PATCH] platform: arm64: qcom-hamoa-ec: reject incomplete responses

[Anvesh Jain P]

On 7/29/2026 3:53 PM, Bryan O'Donoghue wrote:
> On 29/07/2026 11:08, Anvesh Jain P wrote:
>>> -    if (resp[0] >= resp_len)
>>> +    if (resp[0] != resp_len - 1)
>>>           return -EINVAL;
>>>         return 0;
>> Thanks for catching this — nice find.
>>
>> This is independently fixed in an in-flight series that reworks
>> qcom_ec_read() to use raw i2c_transfer() instead of
>> i2c_smbus_read_i2c_block_data(), removing the partial-read case
>> entirely, and it already has the same resp[0] != resp_len - 1 check.
>> That series just hasn't landed yet, so your report stands on its own —
>> happy to add a Reported-by: Linmao Li<lilinmao@kylinos.cn> tag on that
>> commit when it's re-posted, if that works for you.
> 
> What about -stable ?
> 
> Seems like we should take this fix and your series should rebase on top
> of the fix instead of the corollary.
> 
> That's the way this should work Fixes > New stuff.
> 
> Can you add your Review-by/Tested-by and then rebase yourself ?
> 
> ---
> bod

Agreed.

I'll rebase my series on top once this is applied.

Reviewed-by: Anvesh Jain P anvesh.p@oss.qualcomm.com

-- 
Best Regards,
Anvesh

Re: [PATCH] platform: arm64: qcom-hamoa-ec: reject incomplete responses

[Ilpo Järvinen]

On Tue, 28 Jul 2026 19:19:24 +0800, Linmao Li wrote:

> qcom_ec_read() accepts short positive transfers, while both callers
> unconditionally consume every field in their fixed-size response. A short
> transfer can therefore make them use trailing stack bytes that were not
> returned by the device.
> 
> The first response byte contains the number of payload bytes, excluding
> the byte count itself. A complete response of resp_len bytes must
> therefore report resp_len - 1 payload bytes. The existing check only
> rejects counts that do not fit in the response buffer and still accepts
> an incomplete payload.
> 
> [...]

Thank you for your contribution, it has been applied to my local
review-ilpo-next branch. Note it will show up in the public
platform-drivers-x86/review-ilpo-next branch only once I've pushed my
local branch there, which might take a while.

FYI [if applicable to your patch], as per Linus' policy change, also
fixes are mostly routed through for-next unless the fix is for a
commit introduced in the most recent cycle or is clearly a regression
fix.

The list of commits applied:
[1/1] platform: arm64: qcom-hamoa-ec: reject incomplete responses
      commit: 1b04f556e8428a3865a95627ab7a0acb5d9b93a5

--
 i.