Re: [PATCH v6 3/5] thermal/drivers/sun8i: get calibration data from two nvmem cells

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Devres-managed memory allocated for `caldata` is leaked for the lifetime of the device if a secondary nvmem cell fails to be acquired.
--

commit 340e56882111780a9b5ca525d954f99c45894fc7
Author: Mikhail Kalashnikov <iuncuim@gmail.com>

thermal/drivers/sun8i: get calibration data from two nvmem cells

The commit updates the sun8i thermal driver to support retrieving calibration
data from multiple nvmem cells. It iterates over nvmem-cell-names and
concatenates the cell data into a single devm-managed buffer.

> diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c
> index 1412bce630c0c..3606548801f02 100644
> --- a/drivers/thermal/sun8i_thermal.c
> +++ b/drivers/thermal/sun8i_thermal.c
> @@ -300,43 +300,68 @@ static int sun50i_h6_ths_calibrate(struct ths_device *tmdev,
>  
>  static int sun8i_ths_calibrate(struct ths_device *tmdev)
>  {
[ ... ]
> +	of_property_for_each_string(np, "nvmem-cell-names", prop, cellname) {
> +		size_t len;
> +		u8 *caldatapart;
> +
> +		calcell = of_nvmem_cell_get(np, cellname);
> +		if (IS_ERR(calcell)) {
> +			if (PTR_ERR(calcell) == -EPROBE_DEFER)
> +				return -EPROBE_DEFER;
[ ... ]
> +			goto out;

[Severity: Medium]
Does this goto out cause a memory leak of the devres-managed caldata buffer?

If of_nvmem_cell_get() fails for a secondary cell (with an error other than
-EPROBE_DEFER), the function jumps to out returning 0, since ret is not
updated. Since probe succeeds, the devres memory allocated via
devm_krealloc() for the first cell's data is retained for the lifetime of
the device, skipping the explicit devm_kfree(dev, caldata) before the
out label.

> +		}
> +
> +		caldatapart = nvmem_cell_read(calcell, &len);
> +		nvmem_cell_put(calcell);
> +		calcell = NULL;
> +		if (IS_ERR(caldatapart)) {
> +			ret = PTR_ERR(caldatapart);
> +			goto out;
> +		}
> +
> +		caldata = devm_krealloc(dev, caldata, callen + len, GFP_KERNEL);
> +		if (!caldata) {
> +			kfree(caldatapart);
> +			ret = -ENOMEM;
> +			goto out;
> +		}
> +
> +		memcpy(caldata + callen, caldatapart, len);
> +		callen += len;
> +		kfree(caldatapart);
>  	}
>  
> -	caldata = nvmem_cell_read(calcell, &callen);
> -	if (IS_ERR(caldata)) {
> -		ret = PTR_ERR(caldata);
> +	if (!caldata)
>  		goto out;
> -	}
>  
> -	tmdev->chip->calibrate(tmdev, caldata, callen);
> +	tmdev->chip->calibrate(tmdev, (u16 *)caldata, callen);
>  
> -	kfree(caldata);
> +	devm_kfree(dev, caldata);
> +	caldata = NULL;
>  out:
> -	if (!IS_ERR(calcell))
> +	if (!IS_ERR_OR_NULL(calcell))
>  		nvmem_cell_put(calcell);
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913145527.18148-1-iuncuim@gmail.com?part=3