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

Reply via email to