[PATCH 2/2] HID: picolcd: Avoid framebuffer last-close deadlock
From: Aveline Noir <hidden>
Date: 2026-09-28 11:47:00
Also in:
dri-devel, linux-input, lkml
Subsystem:
hid core layer, picolcd hid driver, the rest · Maintainers:
Jiri Kosina, Benjamin Tissoires, Bruno Prémont, Linus Torvalds
Closing the last framebuffer descriptor can deadlock against PicoLCD's
deferred update worker. fb_release() holds info->lock while waiting for
deferred work, and picolcd_fb_update() takes the same lock. Lockdep
reports the cycle through deferred-work completion and fbdefio_state->lock.
Repeated framebuffer open/write/close with concurrent device destruction
reproduces the hang in a PREEMPT_RT QEMU guest.
Use a private update mutex in the deferred worker instead of info->lock.
Take it in picolcd_set_par() as well to preserve serialization of pixel
format conversion and framebuffer updates. Initialize it before exposing
the framebuffer.
The test with the preceding output-request fix hung in the first round
and reported a circular locking dependency. With this change, all five
rounds completed, including 29 successful framebuffer write cycles and
concurrent LCD, backlight, two LED and UHID destroy operations, without
BUG/WARNING. The original syzkaller reproducer and persistent-open
framebuffer tests also passed again. These are bounded virtual-device
tests; physical hardware and suspend/resume remain untested.
Fixes: 3efc61d95259 ("fbdev: Fix invalid page access after closing
deferred I/O devices")
Signed-off-by: Aveline Noir <redacted>
---
drivers/hid/hid-picolcd.h | 2 ++
drivers/hid/hid-picolcd_fb.c | 33 ++++++++++++++++++++++-----------
2 files changed, 24 insertions(+), 11 deletions(-)
diff --git a/drivers/hid/hid-picolcd.h b/drivers/hid/hid-picolcd.h
index 846a8ceb95..33e6654ca0 100644
--- a/drivers/hid/hid-picolcd.h
+++ b/drivers/hid/hid-picolcd.h@@ -115,6 +115,8 @@ struct picolcd_data { struct picolcd_fb_data { /* Framebuffer stuff */ spinlock_t lock; + /* Deferred I/O runs while fbdefio_state->lock is held. */ + struct mutex update_lock; struct picolcd_data *picolcd; u8 update_rate; u8 bpp;
diff --git a/drivers/hid/hid-picolcd_fb.c b/drivers/hid/hid-picolcd_fb.c
index c17104fd60..258c7c4fa2 100644
--- a/drivers/hid/hid-picolcd_fb.c
+++ b/drivers/hid/hid-picolcd_fb.c@@ -231,7 +231,8 @@ static void picolcd_fb_update(struct fb_info *info) struct picolcd_fb_data *fbdata = info->par; struct picolcd_data *data; - mutex_lock(&info->lock); + /* fb_release() flushes this work while holding info->lock. */ + mutex_lock(&fbdata->update_lock); spin_lock_irqsave(&fbdata->lock, flags); data = !fbdata->ready ? fbdata->picolcd : NULL;
@@ -258,11 +259,11 @@ static void picolcd_fb_update(struct fb_info *info) spin_lock_irqsave(&fbdata->lock, flags); data = fbdata->picolcd; spin_unlock_irqrestore(&fbdata->lock, flags); - mutex_unlock(&info->lock); + mutex_unlock(&fbdata->update_lock); if (!data) return; hid_hw_wait(data->hdev); - mutex_lock(&info->lock); + mutex_lock(&fbdata->update_lock); n = 0; } spin_lock_irqsave(&fbdata->lock, flags);
@@ -277,13 +278,13 @@ static void picolcd_fb_update(struct fb_info *info) spin_lock_irqsave(&fbdata->lock, flags); data = fbdata->picolcd; spin_unlock_irqrestore(&fbdata->lock, flags); - mutex_unlock(&info->lock); + mutex_unlock(&fbdata->update_lock); if (data) hid_hw_wait(data->hdev); return; } out: - mutex_unlock(&info->lock); + mutex_unlock(&fbdata->update_lock); } static int picolcd_fb_blank(int blank, struct fb_info *info)
@@ -332,17 +333,24 @@ static int picolcd_set_par(struct fb_info *info) { struct picolcd_fb_data *fbdata = info->par; u8 *tmp_fb, *o_fb; + int ret = 0; + + mutex_lock(&fbdata->update_lock); if (info->var.bits_per_pixel == fbdata->bpp) - return 0; + goto out; /* switch between 1/8 bit depths */ - if (info->var.bits_per_pixel != 1 && info->var.bits_per_pixel != 8) - return -EINVAL; + if (info->var.bits_per_pixel != 1 && info->var.bits_per_pixel != 8) { + ret = -EINVAL; + goto out; + } o_fb = fbdata->bitmap; tmp_fb = kmalloc_array(PICOLCDFB_SIZE, info->var.bits_per_pixel, GFP_KERNEL); - if (!tmp_fb) - return -ENOMEM; + if (!tmp_fb) { + ret = -ENOMEM; + goto out; + } /* translate FB content to new bits-per-pixel */ if (info->var.bits_per_pixel == 1) {
@@ -369,7 +377,9 @@ static int picolcd_set_par(struct fb_info *info) kfree(tmp_fb); fbdata->bpp = info->var.bits_per_pixel; - return 0; +out: + mutex_unlock(&fbdata->update_lock); + return ret; } static void picolcdfb_ops_damage_range(struct fb_info *info, off_t
off, size_t len)
@@ -506,6 +516,7 @@ int picolcd_init_framebuffer(struct picolcd_data *data) fbdata = info->par; spin_lock_init(&fbdata->lock); + mutex_init(&fbdata->update_lock); fbdata->picolcd = data; fbdata->update_rate = PICOLCDFB_UPDATE_RATE_DEFAULT; fbdata->bpp = picolcdfb_var.bits_per_pixel;
--
2.55.0