Re: staging: most: warning: âmboâ may be used uninitialized in this function

From: Andrey Shvetsov
Date: Fri Mar 18 2016 - 11:57:43 EST


On Fri, Mar 18, 2016 at 01:41:19PM +0100, Geert Uytterhoeven wrote:
> On Fri, Mar 18, 2016 at 6:42 AM, Linux Kernel Mailing List
> <linux-kernel@xxxxxxxxxxxxxxx> wrote:
> > Web: https://git.kernel.org/torvalds/c/f45b0fba43f415f69982df743dfa9b5d1b57785e
> > Commit: f45b0fba43f415f69982df743dfa9b5d1b57785e
> > Parent: b3c9f3c56c41cbebe7804b48ba8e6e484509c2c0
> > Refname: refs/heads/master
> > Author: Christian Gromm <christian.gromm@xxxxxxxxxxxxx>
> > AuthorDate: Tue Dec 22 10:53:06 2015 +0100
> > Committer: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > CommitDate: Sun Feb 7 17:34:58 2016 -0800
> >
> > staging: most: remove stacked_mbo
> >
> > This patch makes use of kfifo_peek and kfifo_skip, which renders the
> > variable stacked_mbo useless. It is therefore removed.
> >
> > Signed-off-by: Christian Gromm <christian.gromm@xxxxxxxxxxxxx>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > ---
> > drivers/staging/most/aim-cdev/cdev.c | 16 +++-------------
> > 1 file changed, 3 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
> > index d9c3f56..0ee2f08 100644
> > --- a/drivers/staging/most/aim-cdev/cdev.c
> > +++ b/drivers/staging/most/aim-cdev/cdev.c
>
> > @@ -249,11 +246,7 @@ aim_read(struct file *filp, char __user *buf, size_t count, loff_t *offset)
> > struct aim_channel *c = filp->private_data;
> >
> > mutex_lock(&c->io_mutex);
> > - if (c->stacked_mbo) {
> > - mbo = c->stacked_mbo;
> > - goto start_copy;
> > - }
> > - while ((!kfifo_out(&c->fifo, &mbo, 1)) && (c->dev)) {
> > + while (c->dev && !kfifo_peek(&c->fifo, &mbo)) {
>
> drivers/staging/most/aim-cdev/cdev.c:241: warning: âmboâ may be used
> uninitialized in this function
>
> From looking at the code, it's not obvious to me if this is a false
> positive or not.
> Can it happen that mbo is not initialized fully, e.g. if less than sizeof(mbo)
> bytes have been read from the kfifo?
>
mbo is not touched by the kfifo_peek in the case where (c->dev == NULL),
but since we protect the c->dev by the io_mutex it remains NULL until
the check after the while loop where we quit.

Looks like the analyzer suspects that c->dev may be changed to a valid
pointer before the second check.

So, it is false positive, but it is worth to initialize the mbo with the
NULL to get rid of the warning.

regards
andy