[PATCH] media: i2c: vgxy61: reject out of range MIPI CSI-2 lane numbers

[Guo Zihao]

vgxy61_tx_from_ep() builds the log2phy and phy2log maps straight from the
lane numbers in the device tree endpoint, using them as array indices:

        log2phy[0] = ep.bus.mipi_csi2.clock_lane;
        phy2log[log2phy[0]] = 0;
        for (l = 1; l < l_nb + 1; l++) {
                log2phy[l] = ep.bus.mipi_csi2.data_lanes[l - 1];
                phy2log[log2phy[l]] = l;
        }

Both arrays hold VGXY61_NB_POLARITIES (5) entries, and neither lane
number is checked against that, so an endpoint with a larger value
writes past the end of the arrays on the stack.

The endpoint parsing just above validates the number of lanes, but not
the lane numbers themselves: l_nb is checked against 1, 2 and 4, while
clock_lane and data_lanes[] are used as-is.

v4l2_fwnode_endpoint_alloc_parse() does not constrain them either: the
only use of clock_lane in v4l2-fwnode.c is a BIT(clock_lane) duplicate
check, which does not reject a value that is merely large.

Reject a clock lane or any data lane that is not below
VGXY61_NB_POLARITIES, with the same dev_err() and goto the lane count
check uses.

No Fixes tag. The arrays and the indexing come from the initial driver
import, 153e4ad44d60 ("media: i2c: Add driver for ST VGXY61 camera
sensor"), and have not been touched since.

Reviewed-by: Liu Chao <liuc63@xxxxxxxxxxxx>
Signed-off-by: Guo Zihao <guozh23@xxxxxxxxxxxx>
---
The lane numbers come from the "clock-lanes" and "data-lanes" properties
of the sensor's endpoint node. Both are read as u32 by the fwnode
helpers, so a value like 5 or 0xffffffff reaches vgxy61_tx_from_ep()
unchanged.

For a module built into a device whose DT the machine owner controls this
is not a trust boundary, so the practical impact is a malformed DT
corrupting the stack rather than an attacker escalating. It is the same
class of fix that the driver already applies to the lane count two lines
above, and that other CSI-2 drivers apply to their lane numbers.

dab65dfbf9c8 is a recent example of this file taking a defensive check
for input it cannot control.

 drivers/media/i2c/vgxy61.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/media/i2c/vgxy61.c b/drivers/media/i2c/vgxy61.c
index 3fb2166c8..ed1205cc6 100644
--- a/drivers/media/i2c/vgxy61.c
+++ b/drivers/media/i2c/vgxy61.c
@@ -1457,9 +1457,20 @@ static int vgxy61_tx_from_ep(struct vgxy61_dev *sensor,
 	}
 
 	/* Build log2phy, phy2log and polarities from ep info */
+	if (ep.bus.mipi_csi2.clock_lane >= VGXY61_NB_POLARITIES) {
+		dev_err(&client->dev, "invalid clock lane %u\n",
+			ep.bus.mipi_csi2.clock_lane);
+		goto error_ep;
+	}
 	log2phy[0] = ep.bus.mipi_csi2.clock_lane;
 	phy2log[log2phy[0]] = 0;
 	for (l = 1; l < l_nb + 1; l++) {
+		if (ep.bus.mipi_csi2.data_lanes[l - 1] >=
+		    VGXY61_NB_POLARITIES) {
+			dev_err(&client->dev, "invalid data lane %u\n",
+				ep.bus.mipi_csi2.data_lanes[l - 1]);
+			goto error_ep;
+		}
 		log2phy[l] = ep.bus.mipi_csi2.data_lanes[l - 1];
 		phy2log[log2phy[l]] = l;
 	}
-- 
2.50.1