Coverity: vgxy61_tx_from_ep(): Memory - corruptions

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):

Addresses-Coverity-ID: 1527258 ("Memory - corruptions")

Note that l starts at 1, 2, or 4, so line 1529 could push it to 5, which is out of bounds, etc...

Thanks for your attention!

Coverity-bot

Hello,

Thank you for your report.

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.

Regards,

Benjamin