Re: [PATCH v3 1/4] clk: qcom: common: Register reset controller only when resets are present

[Imran Shaik <imran.shaik@oss.qualcomm.com>]

On 23-07-2026 09:25 pm, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The local pointer `reset` is used uninitialized when registering GDSCs if the clock controller defines GDSCs but no resets.
> --
> 
> --- Patch [1]: [PATCH v3 1/4] clk: qcom: common: Register reset controller only when resets are present ---
> commit 4dacf2df1c1e317705833daaa92c7514eab60f24
> Author: Imran Shaik <imran.shaik@oss.qualcomm.com>
> 
> clk: qcom: common: Register reset controller only when resets are present
> 
> This commit avoids registering a reset controller if the clock controller
> descriptor does not define any resets. It does this by checking if
> desc->num_resets is non-zero before initializing and registering the
> controller.
> 
>> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
>> --- a/drivers/clk/qcom/common.c
>> +++ b/drivers/clk/qcom/common.c
> [ ... ]
>> @@ -359,17 +359,19 @@ int qcom_cc_really_probe(struct device *dev,
>>   		qcom_cc_clk_regs_configure(dev, desc->driver_data, regmap);
>>   	}
>>   
>> -	reset = &cc->reset;
>> -	reset->rcdev.of_node = dev->of_node;
>> -	reset->rcdev.ops = &qcom_reset_ops;
>> -	reset->rcdev.owner = dev->driver->owner;
>> -	reset->rcdev.nr_resets = desc->num_resets;
>> -	reset->regmap = regmap;
>> -	reset->reset_map = desc->resets;
>> -
>> -	ret = devm_reset_controller_register(dev, &reset->rcdev);
>> -	if (ret)
>> -		goto put_rpm;
>> +	if (desc->num_resets) {
>> +		reset = &cc->reset;
>> +		reset->rcdev.of_node = dev->of_node;
>> +		reset->rcdev.ops = &qcom_reset_ops;
>> +		reset->rcdev.owner = dev->driver->owner;
>> +		reset->rcdev.nr_resets = desc->num_resets;
>> +		reset->regmap = regmap;
>> +		reset->reset_map = desc->resets;
>> +
>> +		ret = devm_reset_controller_register(dev, &reset->rcdev);
>> +		if (ret)
>> +			goto put_rpm;
>> +	}
>>   
>>   	if (desc->gdscs && desc->num_gdscs) {
>>   		scd = devm_kzalloc(dev, sizeof(*scd), GFP_KERNEL);
>>   		if (!scd) {
>>   			ret = -ENOMEM;
>>   			goto put_rpm;
>>   		}
>>   		scd->dev = dev;
>>   		scd->scs = desc->gdscs;
>>   		scd->num = desc->num_gdscs;
>>   		scd->pd_list = cc->pd_list;
>>   		ret = gdsc_register(scd, &reset->rcdev, regmap);
> 
> [Severity: High]
> If a clock controller defines GDSCs but no resets, does this leave the reset
> pointer uninitialized when evaluated here?
> 
> During qcom_cc_really_probe(), the new if (desc->num_resets) block skips the
> initialization of the reset pointer when the clock controller has no resets
> defined.
> 
> However, the pointer arithmetic &reset->rcdev is still evaluated when
> calling gdsc_register(). Passing this uninitialized garbage address could invoke
> undefined behavior and cause a panic during boot when compiler hardening tools
> are enabled.
> 

This appears to be a valid issue. I will check further and submit the
updated changes in a separate patch. For now, I'll drop this change from
this series, as it is no longer required after separating AudioCoreCC
clocks and resets into different devices.

Thanks,
Imran