[PATCH v5] drm/ingenic: fix bridge allocation

"H. Nikolaus Schaller" <[email protected]>
Newsgroups org.kernel.vger.stable,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-mips
Message-ID <f77ed4e1547e452668a6549e3966471a6b638a6b.1787593585.git.hns@goldelico.com>
Bridge allocation API has changed and ingenic/drm was broken
leading to

[   54.997593] dw-hdmi-ingenic 10180000.hdmi: Detected HDMI \X controller v1.31a with HDCP (DWC HDMI 3D TX PHY)
[   55.491338] dw-hdmi-ingenic 10180000.hdmi: registered DesignWare HDMI I2C bus driver
[   55.899132] [drm] DRM bridge corrupted or not allocated by devm_drm_bridge_alloc()
[   55.904136] ------------[ cut here ]------------
[   55.908753] WARNING: lib/refcount.c:25 at drm_bridge_get+0x58/0x6c [drm], CPU#0: kworker/u4:2/36
[   55.917538] refcount_t: addition on 0; use-after-free.
...
[   56.354928] [<c04898b8>] drm_bridge_attach+0x80/0x208 [drm]
...

Fixes: 9347f2fbb0183b0 ("drm/bridge: add warning for bridges using neither devm_drm_bridge_alloc() nor drm_bridge_add()")
Tested-by: Waldemar Brodkorb <[email protected]> (on CI20 with HDMI)
Signed-off-by: H. Nikolaus Schaller <[email protected]>
Cc: Waldemar Brodkorb <[email protected]>
Cc: [email protected]
---

Notes:
    v5: fixed a potential issue with directly allocating struct drm_bridge and
        depending on its internal structure. Solve by allocating a stable
        driver-specific container struct ingenic_drm_bridge_alloc that embeds
        a struct drm_bridge.
        Also revisit and adjust object lifecycle by doing drm_bridge_add() before
        drm_bridge_attach().
        Suggested by Sashiko-reviews:
        https://sashiko.dev/#/patchset/1630a544a26fac0b87187885374ebe59fc92df3b.1787575203.git.hns@goldelico.com?part=1
    
    v4: remove setting interlaced mode (would be new feature and not a fix)
        as suggested by [email protected]
    
    v3: fixed a malformed diff in v2
    
    v2: removed ib->bridge->ops = DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_DETECT
        as suggested by Sashiko-reviews:
        https://sashiko.dev/#/patchset/400ba2fe0d4f76484e929d2efaa32f67a940163a.1787477392.git.hns@goldelico.com?part=1

 drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 27 +++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index 42c86f195c66b3..1eeee9d29b6abb 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -122,11 +122,15 @@ struct ingenic_drm {
 
 struct ingenic_drm_bridge {
 	struct drm_encoder encoder;
-	struct drm_bridge bridge, *next_bridge;
+	struct drm_bridge *bridge, *next_bridge;
 
 	struct drm_bus_cfg bus_cfg;
 };
 
+struct ingenic_drm_bridge_alloc {
+	struct drm_bridge bridge;
+};
+
 static inline struct ingenic_drm_bridge *
 to_ingenic_drm_bridge(struct drm_encoder *encoder)
 {
@@ -802,7 +806,7 @@ static int ingenic_drm_bridge_attach(struct drm_bridge *bridge,
 	struct ingenic_drm_bridge *ib = to_ingenic_drm_bridge(encoder);
 
 	return drm_bridge_attach(encoder, ib->next_bridge,
-				 &ib->bridge, flags);
+				 bridge, flags);
 }
 
 static int ingenic_drm_bridge_atomic_check(struct drm_bridge *bridge,
@@ -1107,6 +1111,7 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 	struct clk *parent_clk;
 	struct drm_plane *primary;
 	struct drm_bridge *bridge;
+	struct ingenic_drm_bridge_alloc *bridge_alloc;
 	struct drm_panel *panel;
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
@@ -1314,13 +1319,27 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 
 		drm_encoder_helper_add(encoder, &ingenic_drm_encoder_helper_funcs);
 
-		ib->bridge.funcs = &ingenic_drm_bridge_funcs;
+		bridge_alloc = devm_drm_bridge_alloc(priv->dev,
+						     struct ingenic_drm_bridge_alloc,
+						     bridge,
+						     &ingenic_drm_bridge_funcs);
+		if (IS_ERR(bridge_alloc)) {
+			ret = PTR_ERR(bridge_alloc);
+			goto err_drvdata;
+		}
+
+		ib->bridge = &bridge_alloc->bridge;
+		ib->bridge->of_node = priv->dev->of_node;
+
 		ib->next_bridge = bridge;
 
-		ret = drm_bridge_attach(encoder, &ib->bridge, NULL,
+		drm_bridge_add(ib->bridge);
+
+		ret = drm_bridge_attach(encoder, ib->bridge, NULL,
 					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
 		if (ret) {
 			dev_err(dev, "Unable to attach bridge\n");
+			drm_bridge_remove(ib->bridge);
 			goto err_drvdata;
 		}
 
-- 
2.55.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.