Re: [PATCH 5/8] platform: arm64: qcom-hamoa-ec: Verify required I2C adapter functionality

[Anvesh Jain P <anvesh.p@oss.qualcomm.com>]

On 7/28/2026 11:44 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The IRQ handler is requested and SCI events are enabled before the `ec->ec_cdev` array is allocated, creating a race condition that can lead to a NULL pointer dereference during `qcom_ec_probe()`.
> --
> 
> --- Patch [5]: [PATCH 5/8] platform: arm64: qcom-hamoa-ec: Verify required I2C adapter functionality ---
> commit a6f49e31946f2ee0f2b44aae1db005b085a4d77b
> Author: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
> 
> platform: arm64: qcom-hamoa-ec: Verify required I2C adapter functionality
> 
> Check the underlying I2C adapter supports both the raw I2C transfers
> used by the fan RPM/LUT commands and the SMBus byte and block-write
> transfers used by the other EC commands, and fail probe early with a
> clear error if it doesn't.
> 
>> diff --git a/drivers/platform/arm64/qcom-hamoa-ec.c b/drivers/platform/arm64/qcom-hamoa-ec.c
>> index d0c45d9970eee..a420a180a1948 100644
>> --- a/drivers/platform/arm64/qcom-hamoa-ec.c
>> +++ b/drivers/platform/arm64/qcom-hamoa-ec.c
>> @@ -1195,6 +1195,11 @@ static int qcom_ec_probe(struct i2c_client *client)
>>  	unsigned int i;
>>  	int ret;
>>  
>> +	if (!i2c_check_functionality(client->adapter,
>> +				     I2C_FUNC_I2C | I2C_FUNC_SMBUS_BYTE_DATA |
>> +				     I2C_FUNC_SMBUS_WRITE_I2C_BLOCK))
>> +		return -EOPNOTSUPP;
>> +
>>  	ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
>>  	if (!ec)
>>  		return -ENOMEM;
> [Severity: High]
> Could this regression result in a race condition where the interrupt handler is
> requested and sci events are enabled before the ec->ec_cdev array is
> allocated?
> 
> If an interrupt fires immediately after devm_request_threaded_irq() and
> qcom_ec_sci_evt_control(dev, true) enable it, the handler could execute:
> 
> drivers/platform/arm64/qcom-hamoa-ec.c:qcom_ec_irq()
>   -> qcom_ec_restore_fan_state()
>     -> queues ec->fan_calib_work
> 
> The asynchronous workqueue function would then run:
> 
> drivers/platform/arm64/qcom-hamoa-ec.c:qcom_ec_fan_calib_work_fn() {
>     ...
>     ret = qcom_ec_fan_calibrate(ec, &ec->ec_cdev[i]);
>     ...
> }
> 
> If devm_kcalloc() hasn't yet allocated the ec->ec_cdev array later in
> qcom_ec_probe(), couldn't this cause a null pointer dereference when
> qcom_ec_fan_calibrate() accesses ec_cdev->parent_dev?
>

I'll move the ec_cdev allocation earlier in probe, before the IRQ is
requested and SCI events are enabled, so there's no window for the
handler to fire before it's ready.

-- 
Best Regards,
Anvesh