Re: Coverity: pca953x_gpio_get_multiple(): Uninitialized variables

[Kees Cook <keescook@chromium.org>]

On Fri, Apr 17, 2020 at 06:15:05PM -0400, Paul Thomas wrote:
> On Fri, Apr 17, 2020 at 5:58 PM coverity-bot <keescook@chromium.org> wrote:
> >
> > Hello!
> >
> > This is an experimental semi-automated report about issues detected by
> > Coverity from a scan of next-20200417 as part of the linux-next scan project:
> > https://scan.coverity.com/projects/linux-next-weekly-scan
> >
> > You're getting this email because you were associated with the identified
> > lines of code (noted below) that were touched by commits:
> >
> >   Tue Apr 14 11:28:42 2020 -0400
> >     96d7c7b3e654 ("gpio: gpio-pca953x, Add get_multiple function")
> >
> > Coverity reported the following:
> >
> > *** CID 1492652:  Uninitialized variables  (UNINIT)
> > /drivers/gpio/gpio-pca953x.c: 499 in pca953x_gpio_get_multiple()
> > 493                             if (ret < 0)
> > 494                                     return ret;
> > 495                     }
> > 496                     /* reg_val is relative to the last read byte,
> > 497                      * so only shift the relative bits
> > 498                      */
> > vvv     CID 1492652:  Uninitialized variables  (UNINIT)
> > vvv     Using uninitialized value "reg_val".
> > 499                     value = (reg_val >> (i % 8)) & 0x01;
> > 500                     __assign_bit(i, bits, value);
> > 501             }
> > 502             return ret;
> > 503     }
> > 504
> Well for this case it is forced on the first pass with
>        offset = gc->ngpio;
> so 'i' in the for_each_set_bit() loop will always be at lest 1 less
> than gc->ngpio.
> 
> However, I could see how this is a little are hard for a detection
> tool to follow through:
> offset = gc->ngpio;
>        for_each_set_bit(i, mask, gc->ngpio) {
>                if ((offset >> BANK_SFT) != (i >> BANK_SFT)) {

Ah yeah, it can't see through the bounds of the "if" and offset and
the shifts.

> These tools are very cool, and I'd like fix the detection one way or
> another. Any suggestions on a better syntax?

Well... I don't think it's going to improve its checking of that loop. I
can just mark it false-positive and ignore it. :) (Or you can init
reg_val to zero at the top. *shrug*)

Thanks for looking at it!

-Kees

-- 
Kees Cook