linux-kernel - Re: [PATCH] iio: adc: mt6360: Handle error in cleanup path correctly

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMknhBFm1HPKv6bPnYALc_=wGBHWB3SKnWmnBZ82Pan9z9TPVw@mail.gmail.com>
Date: Mon, 29 Sep 2025 17:26:44 +0200
From: David Lechner <dlechner@...libre.com>
To: Haotian Zhang <vulab@...as.ac.cn>
Cc: Matthias Brugger <matthias.bgg@...il.com>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, 
	Jonathan Cameron <Jonathan.Cameron@...wei.com>, Nuno Sá <nuno.sa@...log.com>, 
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, linux-kernel@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH] iio: adc: mt6360: Handle error in cleanup path correctly

It looks like you missed the IIO mailing list in the CC, so this might
not show up in Jonathan's queue.

On Mon, Sep 29, 2025 at 4:54 AM Haotian Zhang <vulab@...as.ac.cn> wrote:
>
> The return value of a regmap_raw_write() in the cleanup path was
> being ignored.
>
> Fix this by checking the return value and propagating the error.
>
> Fixes: 1f4877218f7e ("iio: adc: mt6360: Add ADC driver for MT6360")
> Signed-off-by: Haotian Zhang <vulab@...as.ac.cn>
> ---
>  drivers/iio/adc/mt6360-adc.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/mt6360-adc.c b/drivers/iio/adc/mt6360-adc.c
> index 69b3569c90e5..97c4af8a93fc 100644
> --- a/drivers/iio/adc/mt6360-adc.c
> +++ b/drivers/iio/adc/mt6360-adc.c
> @@ -70,6 +70,7 @@ static int mt6360_adc_read_channel(struct mt6360_adc_data *mad, int channel, int
>         ktime_t predict_end_t, timeout;
>         unsigned int pre_wait_time;
>         int ret;
> +       int cleanup_ret;
>
>         mutex_lock(&mad->adc_lock);
>
> @@ -130,11 +131,15 @@ static int mt6360_adc_read_channel(struct mt6360_adc_data *mad, int channel, int
>  out_adc_conv:
>         /* Only keep ADC enable */
>         adc_enable = cpu_to_be16(MT6360_ADCEN_MASK);
> -       regmap_raw_write(mad->regmap, MT6360_REG_PMUADCCFG, &adc_enable, sizeof(adc_enable));
> +       cleanup_ret = regmap_raw_write(mad->regmap, MT6360_REG_PMUADCCFG,
> +                               &adc_enable, sizeof(adc_enable));
> +       if (ret >= 0)
> +               ret = cleanup_ret;

This overwrites the original return error, which is IIO_VAL_INT on
success or an error code. In either case, changing the return value
will break things.

In cleanup paths, there really isn't anything we can do with return
values other than ignore them like we were already doing or log an
error message.

>         mad->last_off_timestamps[channel] = ktime_get();
>         /* Config prefer channel to NO_PREFER */
>         regmap_update_bits(mad->regmap, MT6360_REG_PMUADCRPT1, MT6360_PREFERCH_MASK,

The return value here is also being ignored. If we decide that we
should add error messages, then we should add one here as well.

>                            MT6360_NO_PREFER << MT6360_PREFERCH_SHFT);
> +
>  out_adc_lock:
>         mutex_unlock(&mad->adc_lock);
>
> --
> 2.50.1.windows.1
>