* Coverity: pca953x_gpio_get_multiple(): Uninitialized variables
@ 2020-04-17 21:58 coverity-bot
2020-04-17 22:15 ` Paul Thomas
0 siblings, 1 reply; 4+ messages in thread
From: coverity-bot @ 2020-04-17 21:58 UTC (permalink / raw)
To: Paul Thomas
Cc: Linus Walleij, Bartosz Golaszewski, Gustavo A. R. Silva,
linux-next
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
If this is a false positive, please let us know so we can mark it as
such, or teach the Coverity rules to be smarter. If not, please make
sure fixes get into linux-next. :) For patches fixing this, please
include these lines (but double-check the "Fixes" first):
Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
Addresses-Coverity-ID: 1492652 ("Uninitialized variables")
Fixes: 96d7c7b3e654 ("gpio: gpio-pca953x, Add get_multiple function")
Thanks for your attention!
--
Coverity-bot
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: Coverity: pca953x_gpio_get_multiple(): Uninitialized variables
2020-04-17 21:58 Coverity: pca953x_gpio_get_multiple(): Uninitialized variables coverity-bot
@ 2020-04-17 22:15 ` Paul Thomas
2020-04-17 22:45 ` Kees Cook
0 siblings, 1 reply; 4+ messages in thread
From: Paul Thomas @ 2020-04-17 22:15 UTC (permalink / raw)
To: coverity-bot
Cc: Linus Walleij, Bartosz Golaszewski, Gustavo A. R. Silva,
linux-next
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)) {
These tools are very cool, and I'd like fix the detection one way or
another. Any suggestions on a better syntax?
thanks,
Paul
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: Coverity: pca953x_gpio_get_multiple(): Uninitialized variables
2020-04-17 22:15 ` Paul Thomas
@ 2020-04-17 22:45 ` Kees Cook
2020-04-17 23:17 ` Paul Thomas
0 siblings, 1 reply; 4+ messages in thread
From: Kees Cook @ 2020-04-17 22:45 UTC (permalink / raw)
To: Paul Thomas
Cc: Linus Walleij, Bartosz Golaszewski, Gustavo A. R. Silva,
linux-next
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: Coverity: pca953x_gpio_get_multiple(): Uninitialized variables
2020-04-17 22:45 ` Kees Cook
@ 2020-04-17 23:17 ` Paul Thomas
0 siblings, 0 replies; 4+ messages in thread
From: Paul Thomas @ 2020-04-17 23:17 UTC (permalink / raw)
To: Kees Cook
Cc: Linus Walleij, Bartosz Golaszewski, Gustavo A. R. Silva,
linux-next
On Fri, Apr 17, 2020 at 6:45 PM Kees Cook <keescook@chromium.org> wrote:
>
> 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*)
Yeah, init to zero sounds good to me, one instruction is nothing
compared to what this saves by not doing the same i2c transaction
multiple times.
Bart do you want the original patch updated or something else?
Nice work on Coverity Kees!
thanks,
Paul
>
> Thanks for looking at it!
>
> -Kees
>
> --
> Kees Cook
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-04-17 23:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-17 21:58 Coverity: pca953x_gpio_get_multiple(): Uninitialized variables coverity-bot
2020-04-17 22:15 ` Paul Thomas
2020-04-17 22:45 ` Kees Cook
2020-04-17 23:17 ` Paul Thomas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox