Unplugging (or deauthorizing) a GM12U320 device while it is driving an
output causes the system to freeze. When this happens, other screens
stop updating and USB input (keyboard and mouse) stops working, but
non-USB storage, audio, network and ACPI events are unaffected. Because
the unplug never completes, the only way out is a reboot.
The kernel log shows a NULL pointer dereference on every such unplug:
BUG: kernel NULL pointer dereference, address: 0000000000000030
RIP: 0010:drm_mode_object_put+0x9/0x20
Call Trace:
drm_atomic_helper_commit_crtc_disable
drm_atomic_helper_commit_tail
commit_tail
drm_atomic_helper_commit
drm_atomic_commit
drm_atomic_helper_disable_all
drm_atomic_helper_shutdown
gm12u320_usb_disconnect
usb_unbind_interface
...
usb_set_configuration
usb_deauthorize_device
authorized_store
The call trace above was caused by triggering a deauthorization, but a
physical unplug reaches the same gm12u320_usb_disconnect() path through
hub_event/usb_disconnect().
The task faults while holding the USB device mutex
(usb_set_configuration()) and the DRM modeset locks taken by
drm_atomic_helper_shutdown(). Mutexes are not released when a task dies
in an oops, so they stay locked forever, with the USB hub workqueue
(hub_event) and the DRM framebuffer-removal worker both blocking on
them. This is what freezes the system when the driver crashes.
gm12u320_stop_fb_update() drops the cached upload framebuffer
unconditionally:
old_fb = gm12u320->fb_update.fb;
gm12u320->fb_update.fb = NULL;
...
drm_framebuffer_put(old_fb);
gm12u320->fb_update.fb is legitimately NULL when no update is queued,
for example when the update was already stopped, and
drm_framebuffer_put() does not accept NULL. On unplug,
gm12u320_usb_disconnect() calls drm_atomic_helper_shutdown(), which
commits a CRTC disable and reaches gm12u320_stop_fb_update() with a NULL
fb.
The NULL check was lost in commit 8f2cb9379fb4 ("drm/gm12u320: Simplify
upload work"). That commit moved the reference release out of
fb_update.lock (so the framebuffer destructor does not run under the
lock), but the previous form guarded the call with "if
(gm12u320->fb_update.fb)" and that check was dropped in the rewrite.
gm12u320_fb_mark_dirty() still has the same guard.
To fix the crash, restore the NULL check before calling
drm_framebuffer_put(). With this, the CRTC disable completes, no locks
are leaked, and unplugging the GM12U320 device while it is in use no
longer freezes the system. Tested by surprise-unplugging the device as
the active output--before the change the kernel oopsed and wedged on
every attempt, after it there is no oops and no stuck task.
Fixes: 8f2cb9379fb4 ("drm/gm12u320: Simplify upload work")
Cc: [email protected]
Signed-off-by: Forest Crossman <[email protected]>
---
drivers/gpu/drm/tiny/gm12u320.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
index 4ad074337af0..5bb6488556de 100644
--- a/drivers/gpu/drm/tiny/gm12u320.c
+++ b/drivers/gpu/drm/tiny/gm12u320.c
@@ -446,7 +446,8 @@ static void gm12u320_stop_fb_update(struct gm12u320_device
*gm12u320)
iosys_map_clear(&gm12u320->fb_update.src_map);
mutex_unlock(&gm12u320->fb_update.lock);
- drm_framebuffer_put(old_fb);
+ if (old_fb)
+ drm_framebuffer_put(old_fb);
}
static int gm12u320_set_ecomode(struct gm12u320_device *gm12u320)
base-commit: 1717fcc5be575d4768279148ae9465a8b13d4339
--
2.55.0
