linux-kernel - [PATCH 2/2] drm_atomic_private_obj_fini: protect private

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20251013-drm-bridge-atomic-vs-remove-private_obj-v1-2-1fc2e58102e0@bootlin.com>
Date: Mon, 13 Oct 2025 18:24:23 +0200
From: Luca Ceresoli <luca.ceresoli@...tlin.com>
To: Harry Wentland <harry.wentland@....com>, Leo Li <sunpeng.li@....com>, 
 Rodrigo Siqueira <siqueira@...lia.com>, 
 Alex Deucher <alexander.deucher@....com>, 
 Christian König <christian.koenig@....com>, 
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, 
 Liviu Dudau <liviu.dudau@....com>, 
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, 
 Andrzej Hajda <andrzej.hajda@...el.com>, 
 Neil Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>, 
 Laurent Pinchart <Laurent.pinchart@...asonboard.com>, 
 Jonas Karlman <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>, 
 Paul Cercueil <paul@...pouillou.net>, 
 Rob Clark <robin.clark@....qualcomm.com>, 
 Dmitry Baryshkov <lumag@...nel.org>, 
 Abhinav Kumar <abhinav.kumar@...ux.dev>, 
 Jessica Zhang <jessica.zhang@....qualcomm.com>, Sean Paul <sean@...rly.run>, 
 Marijn Suijten <marijn.suijten@...ainline.org>, 
 Tomi Valkeinen <tomi.valkeinen@...asonboard.com>, 
 Thierry Reding <thierry.reding@...il.com>, 
 Mikko Perttunen <mperttunen@...dia.com>, 
 Jonathan Hunter <jonathanh@...dia.com>, 
 Dave Stevenson <dave.stevenson@...pberrypi.com>, 
 Maíra Canal <mcanal@...lia.com>, 
 Raspberry Pi Kernel Maintenance <kernel-list@...pberrypi.com>
Cc: Hui Pu <Hui.Pu@...ealthcare.com>, 
 Thomas Petazzoni <thomas.petazzoni@...tlin.com>, 
 amd-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org, 
 linux-kernel@...r.kernel.org, linux-mips@...r.kernel.org, 
 linux-arm-msm@...r.kernel.org, freedreno@...ts.freedesktop.org, 
 linux-tegra@...r.kernel.org, Luca Ceresoli <luca.ceresoli@...tlin.com>
Subject: [PATCH 2/2] drm_atomic_private_obj_fini: protect private_obj
 removal from list

Currently drm_bridge_detach() expects that the bridge private_obj is not
locked by a drm_modeset_acquire_ctx, and it warns in case that happens:

  drm_bridge_detach()
  -> drm_atomic_private_obj_fini()
     -> list_del(&obj->head) // removes priv_obj from
                             // dev->mode_config.privobj_list
     -> obj->funcs->atomic_destroy_state()
     -> drm_modeset_lock_fini(&obj->lock)
        -> WARN_ON(!list_empty(&lock->head)) // warn if priv_obj->lock
	                                     // is still in ctx->locked

The expectation is not respected when adding bridge hot-plugging. In such
case the warning triggers if the bridge is being removed concurrently to an
operation that locks the private object using a drm_modeset_acquire_ctx,
such as in this execution scenario:

  CPU0:
  drm_mode_obj_get_properties_ioctl() // userspace request
  -> DRM_MODESET_LOCK_ALL_BEGIN()
  .  -> drm_for_each_privobj() // loop on dev->mode_config.privobj_list
  .     - lock the privobj mutex
  .	- add priv_obj->lock to ctx->locked
  .	  (list of locks to be released)
  .
  .                         CPU1:
  .                         drm_bridge_detach() // bridge hot-unplug
  .		            -> WARN triggers!
  .
  -> DRM_MODESET_LOCK_ALL_END()
     -> for each lock in ctx->locked
	- remove priv_obj->lock from ctx->locked
        - unlock the privobj mutex

Fix this by using DRM_MODESET_LOCK_ALL_BEGIN/END() around the list removal
in drm_atomic_private_obj_fini(). This ensures that exactly one of these
happens:

 * the concurrent code (e.g. drm_mode_obj_get_properties_ioctl()) acquires
   all the locks first, so it can execute fully and release the
   privobj->lock before drm_atomic_private_obj_fini() calls list_del() and
   before the WARN_ON()
 * drm_atomic_private_obj_fini() acquires all the locks first, so it
   removes its privobj->lock from the dev->mode_config.privobj_list; the
   concurrent code will run afterwards and not acquire that lock because it
   is not present anymore

Signed-off-by: Luca Ceresoli <luca.ceresoli@...tlin.com>
---
 drivers/gpu/drm/drm_atomic.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 7910dacb269c03a0f3e1785bb864d228a693a1aa..aa13389a8efe06b0f67cdce4699d403906b282be 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -810,7 +810,13 @@ void
 drm_atomic_private_obj_fini(struct drm_device *dev,
 			    struct drm_private_obj *obj)
 {
+	struct drm_modeset_acquire_ctx ctx;
+	int ret = 0;
+
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 	list_del(&obj->head);
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+
 	obj->funcs->atomic_destroy_state(obj, obj->state);
 	drm_modeset_lock_fini(&obj->lock);
 }

-- 
2.51.0