[PATCH] iio: adc: meson-saradc: fix calibration buffer leak on error

[PATCH] iio: adc: meson-saradc: fix calibration buffer leak on error

[Felix Gu]

meson_sar_adc_temp_sensor_init() allocates a buffer with
nvmem_cell_read(), but the old code leaked it if
syscon_regmap_lookup_by_phandle() failed.

Switch buf to __free(kfree) so all return paths release it.

Fixes: d6f2eac64403 ("iio: adc: meson: no devm for nvmem_cell_get")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
 drivers/iio/adc/meson_saradc.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 23991a3612bd..9708ddcc4919 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -786,7 +786,7 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
 static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
-	u8 *buf, trimming_bits, trimming_mask, upper_adc_val;
+	u8 trimming_bits, trimming_mask, upper_adc_val;
 	struct device *dev = indio_dev->dev.parent;
 	struct nvmem_cell *temperature_calib;
 	size_t read_len;
@@ -807,14 +807,12 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
 	}
 
 	read_len = MESON_SAR_ADC_EFUSE_BYTES;
-	buf = nvmem_cell_read(temperature_calib, &read_len);
+	u8 *buf __free(kfree) = nvmem_cell_read(temperature_calib, &read_len);
 	nvmem_cell_put(temperature_calib);
 	if (IS_ERR(buf))
 		return dev_err_probe(dev, PTR_ERR(buf), "failed to read temperature_calib cell\n");
-	if (read_len != MESON_SAR_ADC_EFUSE_BYTES) {
-		kfree(buf);
+	if (read_len != MESON_SAR_ADC_EFUSE_BYTES)
 		return dev_err_probe(dev, -EINVAL, "invalid read size of temperature_calib cell\n");
-	}
 
 	priv->tsc_regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "amlogic,hhi-sysctrl");
 	if (IS_ERR(priv->tsc_regmap))
@@ -835,8 +833,6 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
 	priv->temperature_sensor_adc_val |= upper_adc_val << BITS_PER_BYTE;
 	priv->temperature_sensor_adc_val >>= trimming_bits;
 
-	kfree(buf);
-
 	return 0;
 }
 

---
base-commit: 7080e32d3f09d8688c4a87d81bdcc71f7f606b16
change-id: 20260425-meson_saradc-54abc52d9b31

Best regards,
-- 
Felix Gu <ustc.gu@gmail.com>

Re: [PATCH] iio: adc: meson-saradc: fix calibration buffer leak on error

[David Lechner]

On 4/25/26 11:07 AM, Felix Gu wrote:
> meson_sar_adc_temp_sensor_init() allocates a buffer with
> nvmem_cell_read(), but the old code leaked it if
> syscon_regmap_lookup_by_phandle() failed.
> 
> Switch buf to __free(kfree) so all return paths release it.
> 
> Fixes: d6f2eac64403 ("iio: adc: meson: no devm for nvmem_cell_get")
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> ---
>  drivers/iio/adc/meson_saradc.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index 23991a3612bd..9708ddcc4919 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -786,7 +786,7 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
>  static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
>  {

Nice to see one of these cleanup.h patches that is actually fixing a bug.

Should `#include <linux/cleanup.h>` though rather that relying on it being
included through another header.

With that fixed...

Reviewed-by: David Lechner <dlechner@baylibre.com>

Re: [PATCH] iio: adc: meson-saradc: fix calibration buffer leak on error

[Jonathan Cameron]

On Sat, 25 Apr 2026 11:17:18 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 4/25/26 11:07 AM, Felix Gu wrote:
> > meson_sar_adc_temp_sensor_init() allocates a buffer with
> > nvmem_cell_read(), but the old code leaked it if
> > syscon_regmap_lookup_by_phandle() failed.
> > 
> > Switch buf to __free(kfree) so all return paths release it.
> > 
> > Fixes: d6f2eac64403 ("iio: adc: meson: no devm for nvmem_cell_get")
> > Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> > ---
> >  drivers/iio/adc/meson_saradc.c | 10 +++-------
> >  1 file changed, 3 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> > index 23991a3612bd..9708ddcc4919 100644
> > --- a/drivers/iio/adc/meson_saradc.c
> > +++ b/drivers/iio/adc/meson_saradc.c
> > @@ -786,7 +786,7 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
> >  static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
> >  {  
> 
> Nice to see one of these cleanup.h patches that is actually fixing a bug.

It's not fixing anything as far as I can see.
The syscon_regmap_lookup_by_handle() is earlier in the function and
nvmem_cell_read() isn't called if that fails.

So this is just a code simplification and so small benefit if anything.

Maybe there is an old version that does things in a different order?


> 
> Should `#include <linux/cleanup.h>` though rather that relying on it being
> included through another header.
> 
> With that fixed...
> 
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> 

Re: [PATCH] iio: adc: meson-saradc: fix calibration buffer leak on error

[Rosen Penev]

On Sat, Apr 25, 2026 at 9:07 AM Felix Gu <ustc.gu@gmail.com> wrote:
>
> meson_sar_adc_temp_sensor_init() allocates a buffer with
> nvmem_cell_read(), but the old code leaked it if
> syscon_regmap_lookup_by_phandle() failed.
>
> Switch buf to __free(kfree) so all return paths release it.
>
> Fixes: d6f2eac64403 ("iio: adc: meson: no devm for nvmem_cell_get")
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> ---
>  drivers/iio/adc/meson_saradc.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index 23991a3612bd..9708ddcc4919 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -786,7 +786,7 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
>  static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
>  {
>         struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
> -       u8 *buf, trimming_bits, trimming_mask, upper_adc_val;
> +       u8 trimming_bits, trimming_mask, upper_adc_val;
>         struct device *dev = indio_dev->dev.parent;
>         struct nvmem_cell *temperature_calib;
>         size_t read_len;
> @@ -807,14 +807,12 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
>         }
>
>         read_len = MESON_SAR_ADC_EFUSE_BYTES;
> -       buf = nvmem_cell_read(temperature_calib, &read_len);
> +       u8 *buf __free(kfree) = nvmem_cell_read(temperature_calib, &read_len);
>         nvmem_cell_put(temperature_calib);
>         if (IS_ERR(buf))
>                 return dev_err_probe(dev, PTR_ERR(buf), "failed to read temperature_calib cell\n");
> -       if (read_len != MESON_SAR_ADC_EFUSE_BYTES) {
> -               kfree(buf);
I don't like this. Just add the missing kfree.
> +       if (read_len != MESON_SAR_ADC_EFUSE_BYTES)
>                 return dev_err_probe(dev, -EINVAL, "invalid read size of temperature_calib cell\n");
> -       }
>
>         priv->tsc_regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "amlogic,hhi-sysctrl");
>         if (IS_ERR(priv->tsc_regmap))
> @@ -835,8 +833,6 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
>         priv->temperature_sensor_adc_val |= upper_adc_val << BITS_PER_BYTE;
>         priv->temperature_sensor_adc_val >>= trimming_bits;
>
> -       kfree(buf);
> -
>         return 0;
>  }
>
>
> ---
> base-commit: 7080e32d3f09d8688c4a87d81bdcc71f7f606b16
> change-id: 20260425-meson_saradc-54abc52d9b31
>
> Best regards,
> --
> Felix Gu <ustc.gu@gmail.com>
>

Re: [PATCH] iio: adc: meson-saradc: fix calibration buffer leak on error

[David Lechner]

On 4/25/26 1:44 PM, Jonathan Cameron wrote:
> On Sat, 25 Apr 2026 11:17:18 -0500
> David Lechner <dlechner@baylibre.com> wrote:
> 
>> On 4/25/26 11:07 AM, Felix Gu wrote:
>>> meson_sar_adc_temp_sensor_init() allocates a buffer with
>>> nvmem_cell_read(), but the old code leaked it if
>>> syscon_regmap_lookup_by_phandle() failed.
>>>
>>> Switch buf to __free(kfree) so all return paths release it.
>>>
>>> Fixes: d6f2eac64403 ("iio: adc: meson: no devm for nvmem_cell_get")
>>> Signed-off-by: Felix Gu <ustc.gu@gmail.com>
>>> ---
>>>  drivers/iio/adc/meson_saradc.c | 10 +++-------
>>>  1 file changed, 3 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
>>> index 23991a3612bd..9708ddcc4919 100644
>>> --- a/drivers/iio/adc/meson_saradc.c
>>> +++ b/drivers/iio/adc/meson_saradc.c
>>> @@ -786,7 +786,7 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
>>>  static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
>>>  {  
>>
>> Nice to see one of these cleanup.h patches that is actually fixing a bug.
> 
> It's not fixing anything as far as I can see.
> The syscon_regmap_lookup_by_handle() is earlier in the function and
> nvmem_cell_read() isn't called if that fails.
> 
> So this is just a code simplification and so small benefit if anything.
> 
> Maybe there is an old version that does things in a different order?

I just updated iio/testing today and I see return without kfree().

	priv->tsc_regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "amlogic,hhi-sysctrl");
	if (IS_ERR(priv->tsc_regmap))
		return dev_err_probe(dev, PTR_ERR(priv->tsc_regmap),
				     "failed to get amlogic,hhi-sysctrl regmap\n");


> 
> 
>>
>> Should `#include <linux/cleanup.h>` though rather that relying on it being
>> included through another header.
>>
>> With that fixed...
>>
>> Reviewed-by: David Lechner <dlechner@baylibre.com>
>>
> 

Re: [PATCH] iio: adc: meson-saradc: fix calibration buffer leak on error

[Felix Gu]

On Sun, Apr 26, 2026 at 7:16 AM David Lechner <dlechner@baylibre.com> wrote:
> I just updated iio/testing today and I see return without kfree().
>
>         priv->tsc_regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "amlogic,hhi-sysctrl");
>         if (IS_ERR(priv->tsc_regmap))
>                 return dev_err_probe(dev, PTR_ERR(priv->tsc_regmap),
>                                      "failed to get amlogic,hhi-sysctrl regmap\n");
> >> Should `#include <linux/cleanup.h>` though rather that relying on it being
> >> included through another header.
> >>
> >> With that fixed...
> >>
> >> Reviewed-by: David Lechner <dlechner@baylibre.com>
> >>
> >
>
Hi Jonathan,
This code was merged into linux/master yesterday, perhaps you haven't
updated the code base.

Best regards,
Felix

Re: [PATCH] iio: adc: meson-saradc: fix calibration buffer leak on error

[Jonathan Cameron]

On Sun, 26 Apr 2026 11:54:47 +0800
Felix Gu <ustc.gu@gmail.com> wrote:

> On Sun, Apr 26, 2026 at 7:16 AM David Lechner <dlechner@baylibre.com> wrote:
> > I just updated iio/testing today and I see return without kfree().
> >
> >         priv->tsc_regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "amlogic,hhi-sysctrl");
> >         if (IS_ERR(priv->tsc_regmap))
> >                 return dev_err_probe(dev, PTR_ERR(priv->tsc_regmap),
> >                                      "failed to get amlogic,hhi-sysctrl regmap\n");  
> > >> Should `#include <linux/cleanup.h>` though rather that relying on it being
> > >> included through another header.
> > >>
> > >> With that fixed...
> > >>
> > >> Reviewed-by: David Lechner <dlechner@baylibre.com>
> > >>  
> > >  
> >  
> Hi Jonathan,
> This code was merged into linux/master yesterday, perhaps you haven't
> updated the code base.
Ah.  My bad.  I tend to be lazy and use elixir to check stuff like this
and had completely forgotten I merged the patch that broke this :(

J
> 
> Best regards,
> Felix
> 

Re: [PATCH] iio: adc: meson-saradc: fix calibration buffer leak on error

[Jonathan Cameron]

On Sat, 25 Apr 2026 12:20:55 -0700
Rosen Penev <rosenp@gmail.com> wrote:

> On Sat, Apr 25, 2026 at 9:07 AM Felix Gu <ustc.gu@gmail.com> wrote:
> >
> > meson_sar_adc_temp_sensor_init() allocates a buffer with
> > nvmem_cell_read(), but the old code leaked it if
> > syscon_regmap_lookup_by_phandle() failed.
> >
> > Switch buf to __free(kfree) so all return paths release it.
> >
> > Fixes: d6f2eac64403 ("iio: adc: meson: no devm for nvmem_cell_get")
> > Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> > ---
> >  drivers/iio/adc/meson_saradc.c | 10 +++-------
> >  1 file changed, 3 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> > index 23991a3612bd..9708ddcc4919 100644
> > --- a/drivers/iio/adc/meson_saradc.c
> > +++ b/drivers/iio/adc/meson_saradc.c
> > @@ -786,7 +786,7 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
> >  static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
> >  {
> >         struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
> > -       u8 *buf, trimming_bits, trimming_mask, upper_adc_val;
> > +       u8 trimming_bits, trimming_mask, upper_adc_val;
> >         struct device *dev = indio_dev->dev.parent;
> >         struct nvmem_cell *temperature_calib;
> >         size_t read_len;
> > @@ -807,14 +807,12 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
> >         }
> >
> >         read_len = MESON_SAR_ADC_EFUSE_BYTES;
> > -       buf = nvmem_cell_read(temperature_calib, &read_len);
> > +       u8 *buf __free(kfree) = nvmem_cell_read(temperature_calib, &read_len);
> >         nvmem_cell_put(temperature_calib);
> >         if (IS_ERR(buf))
> >                 return dev_err_probe(dev, PTR_ERR(buf), "failed to read temperature_calib cell\n");
> > -       if (read_len != MESON_SAR_ADC_EFUSE_BYTES) {
> > -               kfree(buf);  
> I don't like this. Just add the missing kfree.

That is the minimal fix, so we should probably do that first
even if we then circle back to consider if __free() magic is worth using here.

J
> > +       if (read_len != MESON_SAR_ADC_EFUSE_BYTES)
> >                 return dev_err_probe(dev, -EINVAL, "invalid read size of temperature_calib cell\n");
> > -       }
> >
> >         priv->tsc_regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "amlogic,hhi-sysctrl");
> >         if (IS_ERR(priv->tsc_regmap))
> > @@ -835,8 +833,6 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
> >         priv->temperature_sensor_adc_val |= upper_adc_val << BITS_PER_BYTE;
> >         priv->temperature_sensor_adc_val >>= trimming_bits;
> >
> > -       kfree(buf);
> > -
> >         return 0;
> >  }
> >
> >
> > ---
> > base-commit: 7080e32d3f09d8688c4a87d81bdcc71f7f606b16
> > change-id: 20260425-meson_saradc-54abc52d9b31
> >
> > Best regards,
> > --
> > Felix Gu <ustc.gu@gmail.com>
> >  

Re: [PATCH] iio: adc: meson-saradc: fix calibration buffer leak on error

[Felix Gu]

On Sun, Apr 26, 2026 at 6:41 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> That is the minimal fix, so we should probably do that first
> even if we then circle back to consider if __free() magic is worth using here.
>
> J

Hi Jonathan,
You prefer a minimal fix here?

Best regards,
Felix

Re: [PATCH] iio: adc: meson-saradc: fix calibration buffer leak on error

[Andy Shevchenko]

On Sun, Apr 26, 2026 at 12:07:24AM +0800, Felix Gu wrote:
> meson_sar_adc_temp_sensor_init() allocates a buffer with
> nvmem_cell_read(), but the old code leaked it if
> syscon_regmap_lookup_by_phandle() failed.

> Switch buf to __free(kfree) so all return paths release it.

__free() is defined in cleanup.h which is missing in the driver. Please add the
include to it.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: adc: meson-saradc: fix calibration buffer leak on error

[Jonathan Cameron]

On Sun, 26 Apr 2026 23:26:33 +0800
Felix Gu <ustc.gu@gmail.com> wrote:

> On Sun, Apr 26, 2026 at 6:41 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > That is the minimal fix, so we should probably do that first
> > even if we then circle back to consider if __free() magic is worth using here.
> >
> > J  
> 
> Hi Jonathan,
> You prefer a minimal fix here?
Yes please.  Then if you like we can carry on discussion about whether
__free() is a good change here as a follow up.

That can include the fact that it would have avoided this bug ever being
introduced.

Thanks,

Jonathan

> 
> Best regards,
> Felix