lkml.org 
[lkml]   [2022]   [Nov]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: Coverity: vgxy61_tx_from_ep(): Memory - corruptions
From
Hello,

Thank you for your report.

On 11/10/22 17:27, coverity-bot wrote:
> Hello!
>
> This is an experimental semi-automated report about issues detected by
> Coverity from a scan of next-20221110 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:
>
> Thu Oct 27 14:37:38 2022 +0300
> 153e4ad44d60 ("media: i2c: Add driver for ST VGXY61 camera sensor")
>
> Coverity reported the following:
>
> *** CID 1527258: Memory - corruptions (OVERRUN)
> drivers/media/i2c/st-vgxy61.c:1528 in vgxy61_tx_from_ep()
> 1522 * valid for hardware stuff.
> 1523 */
> 1524 for (p = 0; p < VGXY61_NB_POLARITIES; p++) {
> 1525 if (phy2log[p] != ~0)
> 1526 continue;
> 1527 phy2log[p] = l;
> vvv CID 1527258: Memory - corruptions (OVERRUN)
> vvv Overrunning array "log2phy" of 5 4-byte elements at element index 5 (byte offset 23) using index "l" (which evaluates to 5).
> 1528 log2phy[l] = p;
> 1529 l++;
> 1530 }
> 1531 for (l = 0; l < l_nb + 1; l++)
> 1532 polarities[l] = ep.bus.mipi_csi2.lane_polarities[l];
> 1533
>
> 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: 1527258 ("Memory - corruptions")
> Fixes: 153e4ad44d60 ("media: i2c: Add driver for ST VGXY61 camera sensor")
>
> Note that l starts at 1, 2, or 4, so line 1529 could push it to 5, which
> is out of bounds, etc...

This 'for' loop is tied with the previous one.
The first loop fills log2phy with provided data, and the second one
fills slots that were not provided with hardware valid data. You can see
in the second loop:
if (phy2log[p] != ~0)
continue;
which prevents this loop from filling slots that were already filled,
and at the same time prevents 'l' to be incremented when this is the case.
As 'l' was incremented for each provided data by the first loop, and
incremented for each not provided data by the second loop. At the end of
the second loop l == VGXY61_NB_POLARITIES == 5 (because of the last
'l++'), but will never be used as an index for 'log2phy' because the
loop will end right after.


So if I'm not mistaken, and while hard to catch by tools this could be
understood as a false positive.

I could guard it with to make the checker happy with:
if (p != VGXY61_NB_POLARITIES - 1) l++;
and it would be exactly the same, but I find this kind of unnecessary.


Any thoughts?

Thank you.

>
> Thanks for your attention!
>

--
Regards,

Benjamin

\
 
 \ /
  Last update: 2022-11-14 14:14    [W:0.036 / U:0.296 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site