Re: [PATCH v15 11/22] dt-bindings: media: i2c: max96714: add support for MAX96714R

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unchecked `v4l2_subdev_state_get_format` dereference in `max96714_apply_patgen_timing` leading to NULL pointer dereference and kernel oops.
--

--- Patch [11]: [PATCH v15 11/22] dt-bindings: media: i2c: max96714: add support for MAX96714R ---
commit c4bf3be89835d23de9ff33f00865d0fa24fed633
Author: Cosmin Tanislav <[email protected]>

dt-bindings: media: i2c: max96714: add support for MAX96714R

This commit adds device tree binding support for the MAX96714R, a lower
capability variant of the MAX96714. It operates at a fixed 3Gbps rate in
the forward direction.

> diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max96714.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max96714.yaml
> index 3ace50e11921b..ec6a91c4a51c0 100644
> --- a/Documentation/devicetree/bindings/media/i2c/maxim,max96714.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max96714.yaml
[ ... ]
> @@ -31,6 +33,7 @@ properties:
>        - items:
>            - enum:
>                - maxim,max96714
> +              - maxim,max96714r
>            - const: maxim,max96714f
>  
>    reg:

[Severity: High]
This is a pre-existing issue, but while looking at the corresponding driver
for this device, I noticed a potential NULL pointer dereference that could
be triggered by userspace:

drivers/media/i2c/max96714.c:max96714_enable_streams() {
    ...
	if (!priv->enabled_source_streams)
		max96714_enable_tx_port(priv);

	ret = max96714_apply_patgen(priv, state);
    ...
}

If userspace sets a custom routing table via VIDIOC_SUBDEV_S_ROUTING with a
source stream greater than 0 (and no stream 0), sets a test pattern via
VIDIOC_SUBDEV_S_CTRL, and enables streams for that stream,
max96714_apply_patgen() will be called unconditionally.

This in turn invokes max96714_apply_patgen_timing(), which attempts to fetch
the format for the source pad:

drivers/media/i2c/max96714.c:max96714_apply_patgen_timing() {
    ...
	struct v4l2_mbus_framefmt *fmt =
		v4l2_subdev_state_get_format(state, MAX96714_PAD_SOURCE);
	const u32 h_active = fmt->width;
    ...
}

Because v4l2_subdev_state_get_format() implicitly defaults to stream 0, could
the lookup return NULL since stream 0 doesn't exist in the user-configured
state in this scenario, resulting in a kernel oops when dereferencing
fmt->width?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11