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....@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....@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

Reply via email to