linux-kernel - Re: [PATCH v4] iio: trigger: stm32-timer-trigger: Add check for clk

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CANeGvZUZw3hxVGAUDhWjbWwVgfDnB33KfzitYwQ73YjLQYeqng@mail.gmail.com>
Date: Sat, 23 Nov 2024 17:04:55 -0500
From: Jiasheng Jiang <jiashengjiangcool@...il.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: dlechner@...libre.com, lars@...afoo.de, mcoquelin.stm32@...il.com, 
	alexandre.torgue@...s.st.com, u.kleine-koenig@...libre.com, 
	tgamblin@...libre.com, fabrice.gasnier@...com, benjamin.gaignard@...aro.org, 
	lee@...nel.org, linux-iio@...r.kernel.org, 
	linux-stm32@...md-mailman.stormreply.com, 
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4] iio: trigger: stm32-timer-trigger: Add check for clk_enable()

Hi Jonathan,

On Sat, Nov 23, 2024 at 10:08 AM Jonathan Cameron <jic23@...nel.org> wrote:
>
> On Mon, 11 Nov 2024 22:23:10 +0000
> Jiasheng Jiang <jiashengjiangcool@...il.com> wrote:
>
> > Add check for the return value of clk_enable() in order to catch the
> > potential exception.
> >
> > Signed-off-by: Jiasheng Jiang <jiashengjiangcool@...il.com>
> Hi Jiasheng,
>
>
> Should definitely mention the changes to use guard() to simplify
> the resulting code.

Thanks, I have revised the "v2 -> v3" in the Changelog to clarify the changes.

> One minor comment on the code inline. Otherwise looks good to me.
>
> Thanks,
>
> Jonathan
>
> > ---
> > Changelog:
> >
> > v3 -> v4:
> >
> > 1. Place braces around the case body.
> >
> > v2 -> v3:
> >
> > 1. Simplify code with cleanup helpers.
> >
> > v1 -> v2:
> >
> > 1. Remove unsuitable dev_err_probe().
>
> > @@ -482,6 +484,7 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
> >                                  int val, int val2, long mask)
> >  {
> >       struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> > +     int ret;
> >
> >       switch (mask) {
> >       case IIO_CHAN_INFO_RAW:
> > @@ -491,12 +494,14 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
> >               /* fixed scale */
> >               return -EINVAL;
> >
> > -     case IIO_CHAN_INFO_ENABLE:
> > -             mutex_lock(&priv->lock);
> > +     case IIO_CHAN_INFO_ENABLE: {
> > +             guard(mutex)(&priv->lock);
> >               if (val) {
> >                       if (!priv->enabled) {
> >                               priv->enabled = true;
> > -                             clk_enable(priv->clk);
> > +                             ret = clk_enable(priv->clk);
> > +                             if (ret)
> > +                                     return ret;
> >                       }
> >                       regmap_set_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN);
> >               } else {
> > @@ -506,9 +511,10 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
> >                               clk_disable(priv->clk);
> >                       }
> >               }
> > -             mutex_unlock(&priv->lock);
> > +
> >               return 0;
> >       }
> Add a default for reasons David mentioned and it also makes it visually clear
> that we expect to get in here for other cases but they are all errors.
>         default:
>                 return -EINVAL;
> > +     }
> >
> And drop this return as unreachable.
>
> >       return -EINVAL;
> >  }

Thanks, I have submitted v5 to include a default and remove the return.

-Jiasheng