linux-kernel - Re: [PATCH] iio:proximity:sx9324: Fix hardware gain read/write

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, 27 Mar 2022 16:47:34 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     Stephen Boyd <swboyd@...omium.org>
Cc:     Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        Lars-Peter Clausen <lars@...afoo.de>,
        linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org,
        Gwendal Grignou <gwendal@...omium.org>
Subject: Re: [PATCH] iio:proximity:sx9324: Fix hardware gain read/write

On Tue, 22 Mar 2022 17:57:26 -0400
Stephen Boyd <swboyd@...omium.org> wrote:

> Quoting Jonathan Cameron (2022-03-22 13:38:44)
> > On Mon, 21 Mar 2022 19:36:33 +0100
> > Stephen Boyd <swboyd@...omium.org> wrote:  
> > > Quoting Jonathan Cameron (2022-03-19 08:26:41)  
> > > > On Fri, 18 Mar 2022 13:48:08 -0700
> > > > Stephen Boyd <swboyd@...omium.org> wrote:  
> > > > >
> > > > > diff --git a/drivers/iio/proximity/sx9324.c b/drivers/iio/proximity/sx9324.c
> > > > > index 0d9bbbb50cb4..a3c8e02f5a56 100644
> > > > > --- a/drivers/iio/proximity/sx9324.c
> > > > > +++ b/drivers/iio/proximity/sx9324.c
> > > > > @@ -379,7 +379,10 @@ static int sx9324_read_gain(struct sx_common_data *data,
> > > > >       if (ret)
> > > > >               return ret;
> > > > >
> > > > > -     *val = 1 << FIELD_GET(SX9324_REG_PROX_CTRL0_GAIN_MASK, regval);
> > > > > +     regval = FIELD_GET(SX9324_REG_PROX_CTRL0_GAIN_MASK, regval);
> > > > > +     if (regval)  
> > > >
> > > > If 0 is reserved then I'd return and error code here to indicate
> > > > we don't know what the gain is rather than carrying on regardless.
> > > > Or is this going to cause problems as it will be an ABI change (error
> > > > return possible when it wasn't really before)?
> > > >  
> > >
> > > That sounds OK to me. The driver is only being introduced now so we can
> > > still fix it to reject a gain of 0. Unless 0 should mean "off", i.e.
> > > hardware gain of 1?  
> > No.  I don't think we want to add that sort of fiddly definition.
> > So error is the way to go - I'd forgotten we only just introduced this
> > so no ABI breakage risk.
> >  
> 
> Ok got it. Does the write_gain function also need to reject values
> greater than 8 and less than or equal to 0?

Ah. Yes, it should indeed as the value is coming from userspace
so we shouldn't trust it to be sensible.

Jonathan