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: <20250626195323.6336820c@jic23-huawei>
Date: Thu, 26 Jun 2025 19:53:23 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: David Lechner <dlechner@...libre.com>
Cc: 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,
 Dan Carpenter <dan.carpenter@...aro.org>, Julia Lawall
 <Julia.Lawall@...ia.fr>
Subject: Re: [PATCH v5 2/3] iio: imu: inv_icm42600: add WoM support

> > +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.

> 
> > +               /* turn off accel sensor if not used */
> > +               if (!st->apex.on && !iio_buffer_enabled(indio_dev)) {
> > +                       conf.mode = INV_ICM42600_SENSOR_MODE_OFF;
> > +                       ret = inv_icm42600_set_accel_conf(st, &conf, &sleep_ms);
> > +                       if (ret)
> > +                               break;
> > +               }
> > +       }