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

[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@xiaopeng.com>
Signed-off-by: Guo Zihao <guozh23@xiaopeng.com>
---
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

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

[Benjamin Mugnier]

Hi,

Please add the driver maintainers in copy while submitting a patch so it
pops up on our mail filters. I added Sylvain here.

Le 18/09/2026 à 08:03, Guo Zihao a écrit :
> 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.

Correct me if I'm wrong, but they are checked at line 1453, which is in
the same function by the way :

  l_nb = ep.bus.mipi_csi2.num_data_lanes;
  if (l_nb != 1 && l_nb != 2 && l_nb != 4) {
    dev_err(&client->dev, "invalid data lane number %d\n", l_nb);
    goto error_ep;
  }

So there is not need to double check it after.

Also, it looks like your patch is LLM generated, if this is the case
please disclose it as per [1].

[1] https://docs.kernel.org/process/coding-assistants.html

> 
> 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@xiaopeng.com>
> Signed-off-by: Guo Zihao <guozh23@xiaopeng.com>
> ---
> 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;
>  	}

-- 
Regards,
Benjamin

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

[Guo Zihao]

Hi Benjamin,

Thanks for the review. The maintainers are in copy now.

On the lane checking: l_nb is the number of lanes, while clock_lane and
data_lanes[] are the lane indices. The check at line 1453 accepts
num_data_lanes being 1, 2 or 4, but an endpoint such as

        data-lanes = <1 2 3 99>;

has a valid count of 4 and an out of range index in the last entry. The
indices are stored as unsigned char in struct v4l2_mbus_config_mipi_csi2,
so an endpoint value of 99 arrives as 99, and anything from 5 up writes
past the five element phy2log array.

For clock_lane the write to phy2log[clock_lane] at line 1461 happens
before the log2phy[0] != 0 check at line 1480. vd55g1 and vd56g3, which
build the same kind of map from the same properties, check clock_lane
before the map is built rather than after.

v2 keeps both checks.

On the tooling question: yes, this was written with the help of an LLM,
and v2 carries the Assisted-by tag as the process documentation asks. I
went through the code and the reasoning myself before sending it.

Regards,
Guo

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

[Benjamin Mugnier]

Hi,

Le 22/09/2026 à 08:46, Guo Zihao a écrit :
> Hi Benjamin,
> 
> Thanks for the review. The maintainers are in copy now.
> 
> On the lane checking: l_nb is the number of lanes, while clock_lane and
> data_lanes[] are the lane indices. The check at line 1453 accepts
> num_data_lanes being 1, 2 or 4, but an endpoint such as
> 
>         data-lanes = <1 2 3 99>;
> 
> has a valid count of 4 and an out of range index in the last entry. The
> indices are stored as unsigned char in struct v4l2_mbus_config_mipi_csi2,
> so an endpoint value of 99 arrives as 99, and anything from 5 up writes
> past the five element phy2log array.

Ah you're right, my mistake. I'll review v2.

> 
> For clock_lane the write to phy2log[clock_lane] at line 1461 happens
> before the log2phy[0] != 0 check at line 1480. vd55g1 and vd56g3, which
> build the same kind of map from the same properties, check clock_lane
> before the map is built rather than after.
> 
> v2 keeps both checks.
> 
> On the tooling question: yes, this was written with the help of an LLM,
> and v2 carries the Assisted-by tag as the process documentation asks. I
> went through the code and the reasoning myself before sending it.
> 
> Regards,
> Guo

-- 
Regards,
Benjamin