linux-kernel - Re: [PATCH] iio: ensure ret is initialized to zero before entering do loop

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: <20160906171008.GA1264@localhost>
Date:   Tue, 6 Sep 2016 10:10:09 -0700
From:   Brian Norris <briannorris@...omium.org>
To:     Jonathan Cameron <jic23@...nel.org>
Cc:     Colin King <colin.king@...onical.com>,
        Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iio: ensure ret is initialized to zero before entering
 do loop

Hi,

On Mon, Sep 05, 2016 at 09:03:26PM +0100, Jonathan Cameron wrote:
> On 05/09/16 15:39, Colin King wrote:
> > From: Colin Ian King <colin.king@...onical.com>
> > 
> > A recent fix to iio_buffer_read_first_n_outer removed ret from being set by
> > a return from wait_event_interruptible and also added a continue in a loop
> > which causes the variable ret to not be set when it reaches the end of the
> > loop.  Fix this by initializing ret to zero.
> > 
> > Also remove extraneous white space at the end of the loop.
> > 
> > Fixes: fcf68f3c0bb2a5 ("fix sched WARNING "do not call blocking ops when !TASK_RUNNING")

Not that it really matters, but if the commit is still going to be
amended at all, the subject was "iio: fix ...", not just "fix ...".
Definitely not important though.

> > Signed-off-by: Colin Ian King <colin.king@...onical.com>
> Good find.  Strange that got through 0-day without a warning...
> 
> Cc'd Brian as author of the fix this is fixing.
> Brian can you sanity check this patch as well.

Indeed, looks fine, and works fine:

Tested-by: Brian Norris <briannorris@...omium.org>
Reviewed-by: Brian Norris <briannorris@...omium.org>

Thanks for the fix Colin, and sorry for not noticing that error :(

> Applied to the fixes-togreg branch of iio.git and marked for stable.
> Ah well, another one for the statistics on stable patches that introduce bugs while
> fixing other bugs.
> 
> Pretty unlikely this will be hit I think, but in theory you never know.
> 
> Jonathan
> > ---
> >  drivers/iio/industrialio-buffer.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> > index 49bf9c5..158aaf4 100644
> > --- a/drivers/iio/industrialio-buffer.c
> > +++ b/drivers/iio/industrialio-buffer.c
> > @@ -110,7 +110,7 @@ ssize_t iio_buffer_read_first_n_outer(struct file *filp, char __user *buf,
> >  	DEFINE_WAIT_FUNC(wait, woken_wake_function);
> >  	size_t datum_size;
> >  	size_t to_wait;
> > -	int ret;
> > +	int ret = 0;
> >  
> >  	if (!indio_dev->info)
> >  		return -ENODEV;
> > @@ -153,7 +153,7 @@ ssize_t iio_buffer_read_first_n_outer(struct file *filp, char __user *buf,
> >  		ret = rb->access->read_first_n(rb, n, buf);
> >  		if (ret == 0 && (filp->f_flags & O_NONBLOCK))
> >  			ret = -EAGAIN;
> > -	 } while (ret == 0);
> > +	} while (ret == 0);

Personally, I avoided the temptation to fix the whitespace error in a
bugfix patch. But this does scratch my itch :)

Brian

> >  	remove_wait_queue(&rb->pollq, &wait);
> >  
> >  	return ret;
> > 
>