drm/bridge: lock the encoder chain in scoped for_each loops

drm_for_each_bridge_in_chain_scoped() and
drm_for_each_bridge_in_chain_from() currently get/put the bridge at each
iteration. But they don't protect the encoder chain, so it could change
(bridges added/removed) while some code is iterating over the list
itself. Such code can then derail on incorrect pointers.

To make iterations safe, augment these for_each macros to lock the encoder
chain mutex at the beginning and unlock it at the end of the loop (be it at
the end of the list, or earlier due to a 'break' or 'return' statement).

This change requires more operations when starting and ending the loop. To
avoid making the macros even more complex, move these operations to helper
functions. Also remname some of the existing helper functions for
consistency.

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
Link: https://patch.msgid.link/20260324-drm-bridge-alloc-encoder-chain-mutex-v5-4-8bf786c5c7e6@bootlin.com
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index a8d67bd9ee505fde4b3798f8ddcbde1c5b00ab0f..d6cd0f5af0456ffb8900845b6e68965fe00a0aaa 100644 (file)
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -1457,26 +1457,37 @@ drm_bridge_chain_get_last_bridge(struct drm_encoder *encoder)
                                                      struct drm_bridge, chain_node));
 }
 
-/**
- * drm_bridge_get_next_bridge_and_put - Get the next bridge in the chain
- *                                      and put the previous
- * @bridge: bridge object
- *
- * Same as drm_bridge_get_next_bridge() but additionally puts the @bridge.
- *
- * RETURNS:
- * the next bridge in the chain after @bridge, or NULL if @bridge is the last.
- */
-static inline struct drm_bridge *
-drm_bridge_get_next_bridge_and_put(struct drm_bridge *bridge)
+/* Internal to drm_for_each_bridge_in_chain*() */
+static inline struct drm_bridge *__drm_for_each_bridge_in_chain_next(struct drm_bridge *bridge)
 {
        struct drm_bridge *next = drm_bridge_get_next_bridge(bridge);
 
+       if (!next)
+               mutex_unlock(&bridge->encoder->bridge_chain_mutex);
+
        drm_bridge_put(bridge);
 
        return next;
 }
 
+/* Internal to drm_for_each_bridge_in_chain*() */
+DEFINE_FREE(__drm_for_each_bridge_in_chain_cleanup, struct drm_bridge *,
+       if (_T) { mutex_unlock(&_T->encoder->bridge_chain_mutex); drm_bridge_put(_T); })
+
+/* Internal to drm_for_each_bridge_in_chain_scoped() */
+static inline struct drm_bridge *
+__drm_for_each_bridge_in_chain_scoped_start(struct drm_encoder *encoder)
+{
+       mutex_lock(&encoder->bridge_chain_mutex);
+
+       struct drm_bridge *bridge = drm_bridge_chain_get_first_bridge(encoder);
+
+       if (!bridge)
+               mutex_unlock(&encoder->bridge_chain_mutex);
+
+       return bridge;
+}
+
 /**
  * drm_for_each_bridge_in_chain_scoped - iterate over all bridges attached
  *                                       to an encoder
@@ -1486,14 +1497,24 @@ drm_bridge_get_next_bridge_and_put(struct drm_bridge *bridge)
  *
  * Iterate over all bridges present in the bridge chain attached to @encoder.
  *
- * Automatically gets/puts the bridge reference while iterating, and puts
- * the reference even if returning or breaking in the middle of the loop.
+ * Automatically gets/puts the bridge reference while iterating and locks
+ * the encoder chain mutex to prevent chain modifications while iterating.
  */
-#define drm_for_each_bridge_in_chain_scoped(encoder, bridge)           \
-       for (struct drm_bridge *bridge __free(drm_bridge_put) =         \
-            drm_bridge_chain_get_first_bridge(encoder);                \
-            bridge;                                                    \
-            bridge = drm_bridge_get_next_bridge_and_put(bridge))
+#define drm_for_each_bridge_in_chain_scoped(encoder, bridge)                           \
+       for (struct drm_bridge *bridge __free(__drm_for_each_bridge_in_chain_cleanup) = \
+               __drm_for_each_bridge_in_chain_scoped_start((encoder));                 \
+            bridge;                                                                    \
+            bridge = __drm_for_each_bridge_in_chain_next(bridge))                      \
+
+/* Internal to drm_for_each_bridge_in_chain_from() */
+static inline struct drm_bridge *
+__drm_for_each_bridge_in_chain_from_start(struct drm_bridge *bridge)
+{
+       drm_bridge_get(bridge);
+       mutex_lock(&bridge->encoder->bridge_chain_mutex);
+
+       return bridge;
+}
 
 /**
  * drm_for_each_bridge_in_chain_from - iterate over all bridges starting
@@ -1505,14 +1526,14 @@ drm_bridge_get_next_bridge_and_put(struct drm_bridge *bridge)
  * Iterate over all bridges in the encoder chain starting from
  * @first_bridge, included.
  *
- * Automatically gets/puts the bridge reference while iterating, and puts
- * the reference even if returning or breaking in the middle of the loop.
+ * Automatically gets/puts the bridge reference while iterating and locks
+ * the encoder chain mutex to prevent chain modifications while iterating.
  */
-#define drm_for_each_bridge_in_chain_from(first_bridge, bridge)                \
-       for (struct drm_bridge *bridge __free(drm_bridge_put) =         \
-                    drm_bridge_get(first_bridge);                      \
-            bridge;                                                    \
-            bridge = drm_bridge_get_next_bridge_and_put(bridge))
+#define drm_for_each_bridge_in_chain_from(first_bridge, bridge)                                \
+       for (struct drm_bridge *bridge __free(__drm_for_each_bridge_in_chain_cleanup) = \
+               __drm_for_each_bridge_in_chain_from_start(first_bridge);                \
+            bridge;                                                                    \
+            bridge = __drm_for_each_bridge_in_chain_next(bridge))                      \
 
 enum drm_mode_status
 drm_bridge_chain_mode_valid(struct drm_bridge *bridge,