linux-kernel - Re: [PATCH v5 2/3] iio: imu: inv

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <dcf86acc-567e-48e3-ad15-fd9522b46180@sabinyo.mountain>
Date: Thu, 26 Jun 2025 14:48:10 -0500
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Jonathan Cameron <jic23@...nel.org>
Cc: David Lechner <dlechner@...libre.com>, jean-baptiste.maneyrol@....com,
	Lars-Peter Clausen <lars@...afoo.de>,
	Nuno Sá <nuno.sa@...log.com>,
	Andy Shevchenko <andy@...nel.org>, linux-iio@...r.kernel.org,
	linux-kernel@...r.kernel.org, Julia Lawall <Julia.Lawall@...ia.fr>
Subject: Re: [PATCH v5 2/3] iio: imu: inv_icm42600: add WoM support

On Thu, Jun 26, 2025 at 07:53:23PM +0100, Jonathan Cameron wrote:
> > > +static int inv_icm42600_accel_disable_wom(struct iio_dev *indio_dev)
> > > +{
> > > +       struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> > > +       struct device *pdev = regmap_get_device(st->map);
> > > +       struct inv_icm42600_sensor_conf conf = INV_ICM42600_SENSOR_CONF_INIT;
> > > +       unsigned int sleep_ms = 0;
> > > +       int ret;
> > > +
> > > +       scoped_guard(mutex, &st->lock) {
> > > +               /*
> > > +                * Consider that turning off WoM is always working to avoid
> > > +                * blocking the chip in on mode and prevent going back to sleep.
> > > +                * If there is an error, the chip will anyway go back to sleep
> > > +                * and the feature will not work anymore.
> > > +                */
> > > +               st->apex.wom.enable = false;
> > > +               st->apex.on--;
> > > +               ret = inv_icm42600_disable_wom(st);
> > > +               if (ret)
> > > +                       break;  
> > 
> > The fact that scoped_guard() uses a for loop is an implementation
> > detail so using break here makes this look like improper C code. I
> > think this would be better to split out the protected section to a
> > separate function and just use the regular guard() macro.
> 
> Good catch.  This feels like something we should have some static analysis
> around as we definitely don't want code assuming that implementation.
> 
> +CC Dan / Julia to see if they agree.
> 

I feel like the scoped_guard() macro is so complicated because they
wanted break statements to work as expected...  (As opposed to how I write
half my loop macros using nested for loops so that when I break it only
breaks from the inner loop and corrupts memory).

regards,
dan carpenter