Re: [PATCH v2 67/78] drm/stm: lvds: Switch to atomic_create_state

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

Pre-existing issues:
- [High] The error code assigned during DRM link parsing is silently overwritten, allowing initialization to proceed with `lvds->primary == NULL`, leading to a NULL pointer dereference during clock registration.
--

--- Patch [67]: [PATCH v2 67/78] drm/stm: lvds: Switch to atomic_create_state ---
commit a0c8f236235c34b542e12fb6855c87676c94c0b3
Author: Maxime Ripard <[email protected]>

drm/stm: lvds: Switch to atomic_create_state

The drm_bridge_funcs.atomic_reset callback and its
drm_atomic_helper_bridge_reset() helper are deprecated. Switch to the
atomic_create_state callback and its counterpart.

> diff --git a/drivers/gpu/drm/stm/lvds.c b/drivers/gpu/drm/stm/lvds.c
> index 50a878688e477..90a44e7220573 100644
> --- a/drivers/gpu/drm/stm/lvds.c
> +++ b/drivers/gpu/drm/stm/lvds.c
> @@ -1038,7 +1038,7 @@ static const struct drm_bridge_funcs lvds_bridge_funcs = {
>  	.atomic_disable = lvds_atomic_disable,
>  	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> -	.atomic_reset = drm_atomic_helper_bridge_reset,
> +	.atomic_create_state = drm_atomic_helper_bridge_create_state,
>  };
>  
>  static int lvds_probe(struct platform_device *pdev)

[Severity: High]
This is a pre-existing issue, but I noticed a potential NULL pointer 
dereference in the probe path located just below this change.

In lvds_probe(), if the device tree configuration specifies only a secondary 
single link, lvds->primary is explicitly set to NULL:

drivers/gpu/drm/stm/lvds.c:lvds_probe() {
    ...
		remote = of_get_next_available_child(port2, NULL);
		if (remote) {
			if (of_graph_get_remote_endpoint(remote)) {
				lvds->link_type = LVDS_SINGLE_LINK_SECONDARY;
				lvds->primary = NULL;
				lvds->secondary = &lvds_phy_16ff_secondary;
			} else {
				ret = (ret == -EINVAL) ? -EINVAL : 0;
			}
    ...
}

The probe function then proceeds to call lvds_pixel_clk_register(lvds). 
When the Common Clock Framework invokes the provided recalc_rate callback 
during registration, the callback accesses lvds->primary without checking 
if it is valid:

drivers/gpu/drm/stm/lvds.c:lvds_pixel_clk_recalc_rate() {
    ...
	val = lvds_read(lvds, lvds->primary->base + lvds->primary->ofs.PLLCR2);
    ...
}

Will this cause a kernel panic during driver probe if a valid single-link 
secondary configuration is used, or if an invalid endpoint error is silently 
overwritten?

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