dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/7] drm: Review of mode copies
@ 2022-11-07 19:25 Ville Syrjala
  2022-11-07 19:25 ` [PATCH v2 1/7] drm/amdgpu: Use drm_mode_init() for on-stack modes Ville Syrjala
                   ` (7 more replies)
  0 siblings, 8 replies; 12+ messages in thread
From: Ville Syrjala @ 2022-11-07 19:25 UTC (permalink / raw)
  To: dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Repost of the stragglers from
https://patchwork.freedesktop.org/series/100393/

Note that I didn't rerun the cocci stuff, nor have I had
time to come up with something to inluce the cocci scripts
in the tree. So it's possible this is missing some new
things that have snuck in the meantime.

Ville Syrjälä (7):
  drm/amdgpu: Use drm_mode_init() for on-stack modes
  drm/hisilicon: Use drm_mode_init() for on-stack modes
  drm/msm: Use drm_mode_init() for on-stack modes
  drm/msm: Use drm_mode_copy()
  drm/mtk: Use drm_mode_init() for on-stack modes
  drm/rockchip: Use drm_mode_copy()
  drm/sti: Use drm_mode_copy()

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c    | 3 ++-
 drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c         | 2 +-
 drivers/gpu/drm/mediatek/mtk_hdmi.c                  | 2 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 7 +++++--
 drivers/gpu/drm/msm/dp/dp_display.c                  | 2 +-
 drivers/gpu/drm/rockchip/cdn-dp-core.c               | 2 +-
 drivers/gpu/drm/rockchip/inno_hdmi.c                 | 2 +-
 drivers/gpu/drm/rockchip/rk3066_hdmi.c               | 2 +-
 drivers/gpu/drm/sti/sti_dvo.c                        | 2 +-
 drivers/gpu/drm/sti/sti_hda.c                        | 2 +-
 drivers/gpu/drm/sti/sti_hdmi.c                       | 2 +-
 11 files changed, 16 insertions(+), 12 deletions(-)

-- 
2.37.4


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/7] drm/amdgpu: Use drm_mode_init() for on-stack modes
  2022-11-07 19:25 [PATCH v2 0/7] drm: Review of mode copies Ville Syrjala
@ 2022-11-07 19:25 ` Ville Syrjala
  2022-11-10 15:27   ` Alex Deucher
  2022-11-07 19:25 ` [PATCH v2 2/7] drm/hisilicon: " Ville Syrjala
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjala @ 2022-11-07 19:25 UTC (permalink / raw)
  To: dri-devel; +Cc: Leo Li, Rodrigo Siqueira, Alex Deucher, amd-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Initialize on-stack modes with drm_mode_init() to guarantee
no stack garbage in the list head, or that we aren't copying
over another mode's list head.

Based on the following cocci script, with manual fixups:
@decl@
identifier M;
expression E;
@@
- struct drm_display_mode M = E;
+ struct drm_display_mode M;

@@
identifier decl.M;
expression decl.E;
statement S, S1;
@@
struct drm_display_mode M;
... when != S
+ drm_mode_init(&M, &E);
+
S1

@@
expression decl.E;
@@
- &*E
+ E

Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Leo Li <sunpeng.li@amd.com>
Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index d9940a3c64dd..7fa4b61bc5bf 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5685,7 +5685,7 @@ create_stream_for_sink(struct amdgpu_dm_connector *aconnector,
 	const struct drm_connector_state *con_state =
 		dm_state ? &dm_state->base : NULL;
 	struct dc_stream_state *stream = NULL;
-	struct drm_display_mode mode = *drm_mode;
+	struct drm_display_mode mode;
 	struct drm_display_mode saved_mode;
 	struct drm_display_mode *freesync_mode = NULL;
 	bool native_mode_found = false;
@@ -5699,6 +5699,7 @@ create_stream_for_sink(struct amdgpu_dm_connector *aconnector,
 
 	struct dc_sink *sink = NULL;
 
+	drm_mode_init(&mode, drm_mode);
 	memset(&saved_mode, 0, sizeof(saved_mode));
 
 	if (aconnector == NULL) {
-- 
2.37.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/7] drm/hisilicon: Use drm_mode_init() for on-stack modes
  2022-11-07 19:25 [PATCH v2 0/7] drm: Review of mode copies Ville Syrjala
  2022-11-07 19:25 ` [PATCH v2 1/7] drm/amdgpu: Use drm_mode_init() for on-stack modes Ville Syrjala
@ 2022-11-07 19:25 ` Ville Syrjala
  2022-11-07 19:25 ` [PATCH v2 3/7] drm/msm: " Ville Syrjala
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjala @ 2022-11-07 19:25 UTC (permalink / raw)
  To: dri-devel; +Cc: Xinliang Liu, Tian Tao, Xinwei Kong, John Stultz, Chen Feng

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Initialize on-stack modes with drm_mode_init() to guarantee
no stack garbage in the list head.

Cc: Xinliang Liu <xinliang.liu@linaro.org>
Cc: Tian Tao <tiantao6@hisilicon.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Xinwei Kong <kong.kongxinwei@hisilicon.com>
Cc: Chen Feng <puck.chen@hisilicon.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
index a0d5aa727d58..d9978b79828c 100644
--- a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
+++ b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
@@ -658,7 +658,7 @@ static enum drm_mode_status dsi_encoder_mode_valid(struct drm_encoder *encoder,
 		 * reset adj_mode to the mode value each time,
 		 * so we don't adjust the mode twice
 		 */
-		drm_mode_copy(&adj_mode, mode);
+		drm_mode_init(&adj_mode, mode);
 
 		crtc_funcs = crtc->helper_private;
 		if (crtc_funcs && crtc_funcs->mode_fixup)
-- 
2.37.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 3/7] drm/msm: Use drm_mode_init() for on-stack modes
  2022-11-07 19:25 [PATCH v2 0/7] drm: Review of mode copies Ville Syrjala
  2022-11-07 19:25 ` [PATCH v2 1/7] drm/amdgpu: Use drm_mode_init() for on-stack modes Ville Syrjala
  2022-11-07 19:25 ` [PATCH v2 2/7] drm/hisilicon: " Ville Syrjala
@ 2022-11-07 19:25 ` Ville Syrjala
  2022-11-07 19:25 ` [PATCH v2 4/7] drm/msm: Use drm_mode_copy() Ville Syrjala
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjala @ 2022-11-07 19:25 UTC (permalink / raw)
  To: dri-devel
  Cc: Sean Paul, linux-arm-msm, Abhinav Kumar, Dmitry Baryshkov, freedreno

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Initialize on-stack modes with drm_mode_init() to guarantee
no stack garbage in the list head, or that we aren't copying
over another mode's list head.

Based on the following cocci script, with manual fixups:
@decl@
identifier M;
expression E;
@@
- struct drm_display_mode M = E;
+ struct drm_display_mode M;

@@
identifier decl.M;
expression decl.E;
statement S, S1;
@@
struct drm_display_mode M;
... when != S
+ drm_mode_init(&M, &E);
+
S1

@@
expression decl.E;
@@
- &*E
+ E

Cc: Rob Clark <robdclark@gmail.com>
Cc: Sean Paul <sean@poorly.run>
Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index 2c14646661b7..0f71e8fe7be7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -237,12 +237,13 @@ static void dpu_encoder_phys_vid_setup_timing_engine(
 	unsigned long lock_flags;
 	struct dpu_hw_intf_cfg intf_cfg = { 0 };
 
+	drm_mode_init(&mode, &phys_enc->cached_mode);
+
 	if (!phys_enc->hw_ctl->ops.setup_intf_cfg) {
 		DPU_ERROR("invalid encoder %d\n", phys_enc != NULL);
 		return;
 	}
 
-	mode = phys_enc->cached_mode;
 	if (!phys_enc->hw_intf->ops.setup_timing_gen) {
 		DPU_ERROR("timing engine setup is not supported\n");
 		return;
@@ -634,7 +635,9 @@ static int dpu_encoder_phys_vid_get_frame_count(
 {
 	struct intf_status s = {0};
 	u32 fetch_start = 0;
-	struct drm_display_mode mode = phys_enc->cached_mode;
+	struct drm_display_mode mode;
+
+	drm_mode_init(&mode, &phys_enc->cached_mode);
 
 	if (!dpu_encoder_phys_vid_is_master(phys_enc))
 		return -EINVAL;
-- 
2.37.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 4/7] drm/msm: Use drm_mode_copy()
  2022-11-07 19:25 [PATCH v2 0/7] drm: Review of mode copies Ville Syrjala
                   ` (2 preceding siblings ...)
  2022-11-07 19:25 ` [PATCH v2 3/7] drm/msm: " Ville Syrjala
@ 2022-11-07 19:25 ` Ville Syrjala
  2022-11-07 19:25 ` [PATCH v2 5/7] drm/mtk: Use drm_mode_init() for on-stack modes Ville Syrjala
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjala @ 2022-11-07 19:25 UTC (permalink / raw)
  To: dri-devel
  Cc: Sean Paul, linux-arm-msm, Abhinav Kumar, Dmitry Baryshkov, freedreno

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

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: Rob Clark <robdclark@gmail.com>
Cc: Sean Paul <sean@poorly.run>
Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index a49f6dbbe888..c9d9b384ddd0 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -857,7 +857,7 @@ static int dp_display_set_mode(struct msm_dp *dp_display,
 
 	dp = container_of(dp_display, struct dp_display_private, dp_display);
 
-	dp->panel->dp_mode.drm_mode = mode->drm_mode;
+	drm_mode_copy(&dp->panel->dp_mode.drm_mode, &mode->drm_mode);
 	dp->panel->dp_mode.bpp = mode->bpp;
 	dp->panel->dp_mode.capabilities = mode->capabilities;
 	dp_panel_init_panel_info(dp->panel);
-- 
2.37.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 5/7] drm/mtk: Use drm_mode_init() for on-stack modes
  2022-11-07 19:25 [PATCH v2 0/7] drm: Review of mode copies Ville Syrjala
                   ` (3 preceding siblings ...)
  2022-11-07 19:25 ` [PATCH v2 4/7] drm/msm: Use drm_mode_copy() Ville Syrjala
@ 2022-11-07 19:25 ` Ville Syrjala
  2022-11-07 19:25 ` [PATCH v2 6/7] drm/rockchip: Use drm_mode_copy() Ville Syrjala
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjala @ 2022-11-07 19:25 UTC (permalink / raw)
  To: dri-devel; +Cc: Chun-Kuang Hu

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Initialize on-stack modes with drm_mode_init() to guarantee
no stack garbage in the list head.

Based on the following cocci script, with manual fixups:
@decl@
identifier M;
expression E;
@@
- struct drm_display_mode M = E;
+ struct drm_display_mode M;

@@
identifier decl.M;
expression decl.E;
statement S, S1;
@@
struct drm_display_mode M;
... when != S
+ drm_mode_init(&M, &E);
+
S1

@@
expression decl.E;
@@
- &*E
+ E

Cc: Chun-Kuang Hu <chunkuang.hu@kernel.org>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index 4c80b6896dc3..12fa78f286ff 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -1217,7 +1217,7 @@ static int mtk_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
 	if (next_bridge) {
 		struct drm_display_mode adjusted_mode;
 
-		drm_mode_copy(&adjusted_mode, mode);
+		drm_mode_init(&adjusted_mode, mode);
 		if (!drm_bridge_chain_mode_fixup(next_bridge, mode,
 						 &adjusted_mode))
 			return MODE_BAD;
-- 
2.37.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 6/7] drm/rockchip: Use drm_mode_copy()
  2022-11-07 19:25 [PATCH v2 0/7] drm: Review of mode copies Ville Syrjala
                   ` (4 preceding siblings ...)
  2022-11-07 19:25 ` [PATCH v2 5/7] drm/mtk: Use drm_mode_init() for on-stack modes Ville Syrjala
@ 2022-11-07 19:25 ` Ville Syrjala
  2022-11-07 19:25 ` [PATCH v2 7/7] drm/sti: " Ville Syrjala
  2022-11-08 15:53 ` [PATCH v2 0/7] drm: Review of mode copies Daniel Vetter
  7 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjala @ 2022-11-07 19:25 UTC (permalink / raw)
  To: dri-devel; +Cc: linux-rockchip, Sandy Huang, linux-arm-kernel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Sandy Huang <hjc@rock-chips.com>
Cc: "Heiko Stübner" <heiko@sntech.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-rockchip@lists.infradead.org
---
 drivers/gpu/drm/rockchip/cdn-dp-core.c | 2 +-
 drivers/gpu/drm/rockchip/inno_hdmi.c   | 2 +-
 drivers/gpu/drm/rockchip/rk3066_hdmi.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
index 518ee13b1d6f..8526dda91931 100644
--- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
+++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
@@ -571,7 +571,7 @@ static void cdn_dp_encoder_mode_set(struct drm_encoder *encoder,
 	video->v_sync_polarity = !!(mode->flags & DRM_MODE_FLAG_NVSYNC);
 	video->h_sync_polarity = !!(mode->flags & DRM_MODE_FLAG_NHSYNC);
 
-	memcpy(&dp->mode, adjusted, sizeof(*mode));
+	drm_mode_copy(&dp->mode, adjusted);
 }
 
 static bool cdn_dp_check_link_status(struct cdn_dp_device *dp)
diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c
index 87b2243ea23e..f51774866f41 100644
--- a/drivers/gpu/drm/rockchip/inno_hdmi.c
+++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
@@ -499,7 +499,7 @@ static void inno_hdmi_encoder_mode_set(struct drm_encoder *encoder,
 	inno_hdmi_setup(hdmi, adj_mode);
 
 	/* Store the display mode for plugin/DPMS poweron events */
-	memcpy(&hdmi->previous_mode, adj_mode, sizeof(hdmi->previous_mode));
+	drm_mode_copy(&hdmi->previous_mode, adj_mode);
 }
 
 static void inno_hdmi_encoder_enable(struct drm_encoder *encoder)
diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
index cf2cf51091a3..90145ad96984 100644
--- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
+++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
@@ -395,7 +395,7 @@ rk3066_hdmi_encoder_mode_set(struct drm_encoder *encoder,
 	struct rk3066_hdmi *hdmi = encoder_to_rk3066_hdmi(encoder);
 
 	/* Store the display mode for plugin/DPMS poweron events. */
-	memcpy(&hdmi->previous_mode, adj_mode, sizeof(hdmi->previous_mode));
+	drm_mode_copy(&hdmi->previous_mode, adj_mode);
 }
 
 static void rk3066_hdmi_encoder_enable(struct drm_encoder *encoder)
-- 
2.37.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 7/7] drm/sti: Use drm_mode_copy()
  2022-11-07 19:25 [PATCH v2 0/7] drm: Review of mode copies Ville Syrjala
                   ` (5 preceding siblings ...)
  2022-11-07 19:25 ` [PATCH v2 6/7] drm/rockchip: Use drm_mode_copy() Ville Syrjala
@ 2022-11-07 19:25 ` Ville Syrjala
  2022-11-08 15:53 ` [PATCH v2 0/7] drm: Review of mode copies Daniel Vetter
  7 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjala @ 2022-11-07 19:25 UTC (permalink / raw)
  To: dri-devel; +Cc: Alain Volmat

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

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: Alain Volmat <alain.volmat@foss.st.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/sti/sti_dvo.c  | 2 +-
 drivers/gpu/drm/sti/sti_hda.c  | 2 +-
 drivers/gpu/drm/sti/sti_hdmi.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c
index b6ee8a82e656..f3a5616b7daf 100644
--- a/drivers/gpu/drm/sti/sti_dvo.c
+++ b/drivers/gpu/drm/sti/sti_dvo.c
@@ -288,7 +288,7 @@ static void sti_dvo_set_mode(struct drm_bridge *bridge,
 
 	DRM_DEBUG_DRIVER("\n");
 
-	memcpy(&dvo->mode, mode, sizeof(struct drm_display_mode));
+	drm_mode_copy(&dvo->mode, mode);
 
 	/* According to the path used (main or aux), the dvo clocks should
 	 * have a different parent clock. */
diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c
index 03cc401ed593..ec6656b9ee7c 100644
--- a/drivers/gpu/drm/sti/sti_hda.c
+++ b/drivers/gpu/drm/sti/sti_hda.c
@@ -524,7 +524,7 @@ static void sti_hda_set_mode(struct drm_bridge *bridge,
 
 	DRM_DEBUG_DRIVER("\n");
 
-	memcpy(&hda->mode, mode, sizeof(struct drm_display_mode));
+	drm_mode_copy(&hda->mode, mode);
 
 	if (!hda_get_mode_idx(hda->mode, &mode_idx)) {
 		DRM_ERROR("Undefined mode\n");
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index cb82622877d2..fcc2194869d6 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -941,7 +941,7 @@ static void sti_hdmi_set_mode(struct drm_bridge *bridge,
 	DRM_DEBUG_DRIVER("\n");
 
 	/* Copy the drm display mode in the connector local structure */
-	memcpy(&hdmi->mode, mode, sizeof(struct drm_display_mode));
+	drm_mode_copy(&hdmi->mode, mode);
 
 	/* Update clock framerate according to the selected mode */
 	ret = clk_set_rate(hdmi->clk_pix, mode->clock * 1000);
-- 
2.37.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/7] drm: Review of mode copies
  2022-11-07 19:25 [PATCH v2 0/7] drm: Review of mode copies Ville Syrjala
                   ` (6 preceding siblings ...)
  2022-11-07 19:25 ` [PATCH v2 7/7] drm/sti: " Ville Syrjala
@ 2022-11-08 15:53 ` Daniel Vetter
  2022-11-08 22:13   ` Alex Deucher
  7 siblings, 1 reply; 12+ messages in thread
From: Daniel Vetter @ 2022-11-08 15:53 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: dri-devel

On Mon, Nov 07, 2022 at 09:25:38PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Repost of the stragglers from
> https://patchwork.freedesktop.org/series/100393/
> 
> Note that I didn't rerun the cocci stuff, nor have I had
> time to come up with something to inluce the cocci scripts
> in the tree. So it's possible this is missing some new
> things that have snuck in the meantime.
> 
> Ville Syrjälä (7):
>   drm/amdgpu: Use drm_mode_init() for on-stack modes
>   drm/hisilicon: Use drm_mode_init() for on-stack modes
>   drm/msm: Use drm_mode_init() for on-stack modes
>   drm/msm: Use drm_mode_copy()
>   drm/mtk: Use drm_mode_init() for on-stack modes
>   drm/rockchip: Use drm_mode_copy()
>   drm/sti: Use drm_mode_copy()

On the series:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Imo ping Alex/Rob and stuff everything left into drm-misc-next.
-Daniel

> 
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c    | 3 ++-
>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c         | 2 +-
>  drivers/gpu/drm/mediatek/mtk_hdmi.c                  | 2 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 7 +++++--
>  drivers/gpu/drm/msm/dp/dp_display.c                  | 2 +-
>  drivers/gpu/drm/rockchip/cdn-dp-core.c               | 2 +-
>  drivers/gpu/drm/rockchip/inno_hdmi.c                 | 2 +-
>  drivers/gpu/drm/rockchip/rk3066_hdmi.c               | 2 +-
>  drivers/gpu/drm/sti/sti_dvo.c                        | 2 +-
>  drivers/gpu/drm/sti/sti_hda.c                        | 2 +-
>  drivers/gpu/drm/sti/sti_hdmi.c                       | 2 +-
>  11 files changed, 16 insertions(+), 12 deletions(-)
> 
> -- 
> 2.37.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/7] drm: Review of mode copies
  2022-11-08 15:53 ` [PATCH v2 0/7] drm: Review of mode copies Daniel Vetter
@ 2022-11-08 22:13   ` Alex Deucher
  2022-11-09 11:17     ` Ville Syrjälä
  0 siblings, 1 reply; 12+ messages in thread
From: Alex Deucher @ 2022-11-08 22:13 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel

On Tue, Nov 8, 2022 at 10:54 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Mon, Nov 07, 2022 at 09:25:38PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Repost of the stragglers from
> > https://patchwork.freedesktop.org/series/100393/
> >
> > Note that I didn't rerun the cocci stuff, nor have I had
> > time to come up with something to inluce the cocci scripts
> > in the tree. So it's possible this is missing some new
> > things that have snuck in the meantime.
> >
> > Ville Syrjälä (7):
> >   drm/amdgpu: Use drm_mode_init() for on-stack modes
> >   drm/hisilicon: Use drm_mode_init() for on-stack modes
> >   drm/msm: Use drm_mode_init() for on-stack modes
> >   drm/msm: Use drm_mode_copy()
> >   drm/mtk: Use drm_mode_init() for on-stack modes
> >   drm/rockchip: Use drm_mode_copy()
> >   drm/sti: Use drm_mode_copy()
>
> On the series:
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Imo ping Alex/Rob and stuff everything left into drm-misc-next.

Acked-by: Alex Deucher <alexander.deucher@amd.com>
Feel free to take this series through whatever tree makes sense.  If
you want me to pick up the amdgpu change directly, I can do that too.

Alex

> -Daniel
>
> >
> >  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c    | 3 ++-
> >  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c         | 2 +-
> >  drivers/gpu/drm/mediatek/mtk_hdmi.c                  | 2 +-
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 7 +++++--
> >  drivers/gpu/drm/msm/dp/dp_display.c                  | 2 +-
> >  drivers/gpu/drm/rockchip/cdn-dp-core.c               | 2 +-
> >  drivers/gpu/drm/rockchip/inno_hdmi.c                 | 2 +-
> >  drivers/gpu/drm/rockchip/rk3066_hdmi.c               | 2 +-
> >  drivers/gpu/drm/sti/sti_dvo.c                        | 2 +-
> >  drivers/gpu/drm/sti/sti_hda.c                        | 2 +-
> >  drivers/gpu/drm/sti/sti_hdmi.c                       | 2 +-
> >  11 files changed, 16 insertions(+), 12 deletions(-)
> >
> > --
> > 2.37.4
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/7] drm: Review of mode copies
  2022-11-08 22:13   ` Alex Deucher
@ 2022-11-09 11:17     ` Ville Syrjälä
  0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2022-11-09 11:17 UTC (permalink / raw)
  To: Alex Deucher; +Cc: dri-devel

On Tue, Nov 08, 2022 at 05:13:37PM -0500, Alex Deucher wrote:
> On Tue, Nov 8, 2022 at 10:54 AM Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > On Mon, Nov 07, 2022 at 09:25:38PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Repost of the stragglers from
> > > https://patchwork.freedesktop.org/series/100393/
> > >
> > > Note that I didn't rerun the cocci stuff, nor have I had
> > > time to come up with something to inluce the cocci scripts
> > > in the tree. So it's possible this is missing some new
> > > things that have snuck in the meantime.
> > >
> > > Ville Syrjälä (7):
> > >   drm/amdgpu: Use drm_mode_init() for on-stack modes
> > >   drm/hisilicon: Use drm_mode_init() for on-stack modes
> > >   drm/msm: Use drm_mode_init() for on-stack modes
> > >   drm/msm: Use drm_mode_copy()
> > >   drm/mtk: Use drm_mode_init() for on-stack modes
> > >   drm/rockchip: Use drm_mode_copy()
> > >   drm/sti: Use drm_mode_copy()
> >
> > On the series:
> >
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >
> > Imo ping Alex/Rob and stuff everything left into drm-misc-next.
> 
> Acked-by: Alex Deucher <alexander.deucher@amd.com>
> Feel free to take this series through whatever tree makes sense.  If
> you want me to pick up the amdgpu change directly, I can do that too.

Please do. Less work for me ;)

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/7] drm/amdgpu: Use drm_mode_init() for on-stack modes
  2022-11-07 19:25 ` [PATCH v2 1/7] drm/amdgpu: Use drm_mode_init() for on-stack modes Ville Syrjala
@ 2022-11-10 15:27   ` Alex Deucher
  0 siblings, 0 replies; 12+ messages in thread
From: Alex Deucher @ 2022-11-10 15:27 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: Leo Li, Rodrigo Siqueira, amd-gfx, dri-devel, Alex Deucher

On Mon, Nov 7, 2022 at 2:26 PM Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Initialize on-stack modes with drm_mode_init() to guarantee
> no stack garbage in the list head, or that we aren't copying
> over another mode's list head.
>
> Based on the following cocci script, with manual fixups:
> @decl@
> identifier M;
> expression E;
> @@
> - struct drm_display_mode M = E;
> + struct drm_display_mode M;
>
> @@
> identifier decl.M;
> expression decl.E;
> statement S, S1;
> @@
> struct drm_display_mode M;
> ... when != S
> + drm_mode_init(&M, &E);
> +
> S1
>
> @@
> expression decl.E;
> @@
> - &*E
> + E
>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Leo Li <sunpeng.li@amd.com>
> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Reviewed-by: Harry Wentland <harry.wentland@amd.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Applied.  Thanks!

Alex

> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index d9940a3c64dd..7fa4b61bc5bf 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -5685,7 +5685,7 @@ create_stream_for_sink(struct amdgpu_dm_connector *aconnector,
>         const struct drm_connector_state *con_state =
>                 dm_state ? &dm_state->base : NULL;
>         struct dc_stream_state *stream = NULL;
> -       struct drm_display_mode mode = *drm_mode;
> +       struct drm_display_mode mode;
>         struct drm_display_mode saved_mode;
>         struct drm_display_mode *freesync_mode = NULL;
>         bool native_mode_found = false;
> @@ -5699,6 +5699,7 @@ create_stream_for_sink(struct amdgpu_dm_connector *aconnector,
>
>         struct dc_sink *sink = NULL;
>
> +       drm_mode_init(&mode, drm_mode);
>         memset(&saved_mode, 0, sizeof(saved_mode));
>
>         if (aconnector == NULL) {
> --
> 2.37.4
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-11-10 15:27 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-07 19:25 [PATCH v2 0/7] drm: Review of mode copies Ville Syrjala
2022-11-07 19:25 ` [PATCH v2 1/7] drm/amdgpu: Use drm_mode_init() for on-stack modes Ville Syrjala
2022-11-10 15:27   ` Alex Deucher
2022-11-07 19:25 ` [PATCH v2 2/7] drm/hisilicon: " Ville Syrjala
2022-11-07 19:25 ` [PATCH v2 3/7] drm/msm: " Ville Syrjala
2022-11-07 19:25 ` [PATCH v2 4/7] drm/msm: Use drm_mode_copy() Ville Syrjala
2022-11-07 19:25 ` [PATCH v2 5/7] drm/mtk: Use drm_mode_init() for on-stack modes Ville Syrjala
2022-11-07 19:25 ` [PATCH v2 6/7] drm/rockchip: Use drm_mode_copy() Ville Syrjala
2022-11-07 19:25 ` [PATCH v2 7/7] drm/sti: " Ville Syrjala
2022-11-08 15:53 ` [PATCH v2 0/7] drm: Review of mode copies Daniel Vetter
2022-11-08 22:13   ` Alex Deucher
2022-11-09 11:17     ` Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).