Re: [PATCH] iio: trigger: Fix condition for own trigger

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 16 Jun 2024 11:38:54 +0200
Francesco Dolcini <francesco@dolcini.it> wrote:

> On Sat, Jun 15, 2024 at 11:50:18AM +0100, Jonathan Cameron wrote:
> > On Fri, 14 Jun 2024 11:36:58 -0300
> > João Paulo Gonçalves <jpaulo.silvagoncalves@gmail.com> wrote:
> >   
> > > From: João Paulo Gonçalves <joao.goncalves@toradex.com>
> > > 
> > > The condition for checking if triggers belong to the same IIO device to
> > > set attached_own_device is currently inverted, causing
> > > iio_trigger_using_own() to return an incorrect value. Fix it by testing
> > > for the correct return value of iio_validate_own_trigger().
> > > 
> > > Cc: stable@vger.kernel.org
> > > Fixes: 517985ebc531 ("iio: trigger: Add simple trigger_validation helper")
> > > Signed-off-by: João Paulo Gonçalves <joao.goncalves@toradex.com>  
> 
> Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> 
> > 
> > Ouch.  Can you give an example of resulting user visible result? That
> > will help people decide whether to pick this up for their distro kernels
> > etc.  In some cases, looks like we'll get garbage timestamps and in others
> > may get stale data (or garbage).  
> 
> This was noticed while me and Joao were working on the ads1119 driver you
> have been recently reviewing. We wanted to use iio_trigger_using_own()
> and it was not behaving the right way. We looked into it and found the bug.
> 
> Given that I do not know the exact impact on the drivers that are using this
> function.
> 
> > Odd no one has noticed this in the past whilst testing those dependent
> > features in particular drivers and I worry a little that we may have bugs
> > in the users as a result of iio_trigger_using_own() reporting the inverse
> > of the intended. I've take a quick look at the users and 'think' they are
> > ok, but would definitely like a few others to confirm.  
> 
> All the users of iio_trigger_using_own() are older than the commit that
> introduced the bug, it is safe to assume that they need the fix and
> are expecting the function to behave the same way is documented and it was
> before the bug was introduced.
> 
> The broken commit is not that old and less than 10 IIO drivers are using this
> function. Given that I think that is not that odd that it took 1 year to find
> the bug.

Yes. Long tail of IIO devices that are used on the sort of board that only
gets a kernel update once in a while and well behind mainline.  So indeed
not that surprising :( 

Applied to the fixes-togreg branch of iio.git

Thanks,

Jonathan

> 
> Francesco
>