linux-kernel - Re: [PATCH v3 2/3] iio: add MEMSensing MSA311 3-axis accelerometer driver

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]

Date:   Sun, 19 Jun 2022 13:31:42 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     Dmitry Rokosov <DDRokosov@...rdevices.ru>
Cc:     Andy Shevchenko <andy.shevchenko@...il.com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "stano.jakubek@...il.com" <stano.jakubek@...il.com>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "lars@...afoo.de" <lars@...afoo.de>,
        "stephan@...hold.net" <stephan@...hold.net>,
        "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        kernel <kernel@...rdevices.ru>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 2/3] iio: add MEMSensing MSA311 3-axis accelerometer
 driver

On Thu, 16 Jun 2022 17:02:08 +0000
Dmitry Rokosov <DDRokosov@...rdevices.ru> wrote:



> > > +       err = -EINVAL;
> > > +       mutex_lock(&msa311->lock);
> > > +       for (odr = 0; odr < ARRAY_SIZE(msa311_odr_table); ++odr)
> > > +               if (val == msa311_odr_table[odr].val &&
> > > +                   val2 == msa311_odr_table[odr].val2) {
> > > +                       err = msa311_set_odr(msa311, odr);  
> >   
> > > +                       if (err) {
> > > +                               dev_err(dev, "cannot update freq (%d)\n", err);
> > > +                               goto failed;
> > > +                       }  
> > 
> > Why is this inside the loop and more important under lock? Also you
> > may cover the initial error code by this message when moving it out of
> > the loop and lock.
> > 
> > Ditto for other code snippets in other function(s) where applicable.
> >   
> 
> Yes, I can move dev_err() outside of loop. But all ODR search loop
> should be under lock fully, because other msa311 operations should not
> be executed when we search proper ODR place.

I don't see why?  The search itself is for a match of the input to const data.
That can occur before taking the lock to do the actual write.

I don't see any additional race beyond the one that is always there of
a thread updating ODR whilst another is accessing the device.  Which order
those events happen in is not controlled by the driver, but the output
will be consistent with one or other order of those two accesses.

Jonathan