Hi Ville,
Thank you for the patch.
On Fri, Feb 18, 2022 at 12:03:47PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> struct drm_display_mode embeds a list head, so overwriting
> the full struct with another one will corrupt the list
> (if the destination mode is on a list). Use drm_mode_copy()
> instead which explicitly preserves the list head of
> the destination mode.
>
> Even if we know the destination mode is not on any list
> using drm_mode_copy() seems decent as it sets a good
> example. Bad examples of not using it might eventually
> get copied into code where preserving the list head
> actually matters.
>
> Obviously one case not covered here is when the mode
> itself is embedded in a larger structure and the whole
> structure is copied. But if we are careful when copying
> into modes embedded in structures I think we can be a
> little more reassured that bogus list heads haven't been
> propagated in.
>
> @is_mode_copy@
> @@
> drm_mode_copy(...)
> {
> ...
> }
>
> @depends on !is_mode_copy@
> struct drm_display_mode *mode;
> expression E, S;
> @@
> (
> - *mode = E
> + drm_mode_copy(mode, &E)
> |
> - memcpy(mode, E, S)
> + drm_mode_copy(mode, E)
> )
>
> @depends on !is_mode_copy@
> struct drm_display_mode mode;
> expression E;
> @@
> (
> - mode = E
> + drm_mode_copy(&mode, &E)
> |
> - memcpy(&mode, E, S)
> + drm_mode_copy(&mode, E)
> )
>
> @@
> struct drm_display_mode *mode;
> @@
> - &*mode
> + mode
>
> Cc: Andrzej Hajda <andrzej.hajda@xxxxxxxxx>
> Cc: Neil Armstrong <narmstrong@xxxxxxxxxxxx>
> Cc: Robert Foss <robert.foss@xxxxxxxxxx>
> Cc: Laurent Pinchart <Laurent.pinchart@xxxxxxxxxxxxxxxx>
> Cc: Jonas Karlman <jonas@xxxxxxxxx>
> Cc: Jernej Skrabec <jernej.skrabec@xxxxxxxxx>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/bridge/nwl-dsi.c | 2 +-
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
> drivers/gpu/drm/bridge/tc358767.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c b/drivers/gpu/drm/bridge/nwl-dsi.c
> index 963a6794735f..881cf338d5cf 100644
> --- a/drivers/gpu/drm/bridge/nwl-dsi.c
> +++ b/drivers/gpu/drm/bridge/nwl-dsi.c
> @@ -857,7 +857,7 @@ nwl_dsi_bridge_mode_set(struct drm_bridge *bridge,
> /* Save the new desired phy config */
> memcpy(&dsi->phy_cfg, &new_cfg, sizeof(new_cfg));
>
> - memcpy(&dsi->mode, adjusted_mode, sizeof(dsi->mode));
> + drm_mode_copy(&dsi->mode, adjusted_mode);
> drm_mode_debug_printmodeline(adjusted_mode);
>
> if (pm_runtime_resume_and_get(dev) < 0)
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 4befc104d220..a563460f8d20 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2830,7 +2830,7 @@ static void dw_hdmi_bridge_mode_set(struct drm_bridge *bridge,
> mutex_lock(&hdmi->mutex);
>
> /* Store the display mode for plugin/DKMS poweron events */
> - memcpy(&hdmi->previous_mode, mode, sizeof(hdmi->previous_mode));
> + drm_mode_copy(&hdmi->previous_mode, mode);
>
> mutex_unlock(&hdmi->mutex);
> }
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index c23e0abc65e8..7f9574b17caa 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -1312,7 +1312,7 @@ static void tc_bridge_mode_set(struct drm_bridge *bridge,
> {
> struct tc_data *tc = bridge_to_tc(bridge);
>
> - tc->mode = *mode;
> + drm_mode_copy(&tc->mode, mode);
> }
>
> static struct edid *tc_get_edid(struct drm_bridge *bridge,
--
Regards,
Laurent Pinchart
