From: Yao Yiqi <[email protected]>
fbcon_cursor() can be reached from the console printing path, which runs
in atomic context. When the VT software cursor is enabled
(vc_cursor_type & CUR_SW), it calls fbcon_del_cursor_work(), which uses
cancel_delayed_work_sync() and therefore may sleep.
With CONFIG_DEBUG_ATOMIC_SLEEP enabled, the following is reported:
BUG: sleeping function called from invalid context at kernel/workqueue.c:4722
in_atomic(): 1, irqs_disabled(): 1, non_block: 0
...
__cancel_work_sync
fbcon_del_cursor_work
fbcon_cursor
hide_cursor
vt_console_print
console_unlock
...
Make the default helper safe: fbcon_del_cursor_work() only stops
blinking, using cancel_delayed_work(), which is allowed in any context.
Every "stop blinking" call site keeps using it, including fbcon_cursor().
Only fbcon_release(), which is about to free @par and must ensure that no
callback is still using it, calls the separate
fbcon_del_cursor_work_sync(). New call sites therefore get the safe
variant by default.
cancel_delayed_work() does not wait for a running callback, and
fb_flashcursor() re-arms itself, so also move the requeue into the
console_lock section. fbcon_del_cursor_work() callers hold
console_lock, which serializes them with the requeue and keeps the
non-synchronizing cancel effective.
Fixes: 3b0fb6ab25dd ("fbcon: Use delayed work for cursor")
Cc: [email protected]
Assisted-by: Opencode:deepseek-v4.1-flash
Signed-off-by: Yao Yiqi <[email protected]>
---
Sorry for the last email, I forgot to write changelog. This email has no
code modification, it just adds the changelog.
Changes in v2:
- Reworked per review (sashiko-AI): do not guess the context with
in_interrupt()/irqs_disabled(). fbcon_del_cursor_work() is now
non-synchronizing by default and therefore safe from any context;
fbcon_del_cursor_work_sync() is added for fbcon_release(), the only
path that frees @par and needs to drain the work.
- Move the requeue in fb_flashcursor() under console_lock so a running
callback cannot re-arm the work after the non-synchronizing cancel.
drivers/video/fbdev/core/fbcon.c | 28 ++++++++++++++++++++++++++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 94abbb5ac766..917ce9d641a1 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -422,10 +422,14 @@ static void fb_flashcursor(struct work_struct *work)
par->bitops->cursor(vc, info, enable,
get_fg_color(vc, info, c),
get_bg_color(vc, info, c));
- console_unlock();
+ /*
+ * Re-arm under console_lock so that a concurrent
+ * fbcon_del_cursor_work() cannot race with this requeue.
+ */
queue_delayed_work(system_power_efficient_wq, &par->cursor_work,
par->cur_blink_jiffies);
+ console_unlock();
}
static void fbcon_add_cursor_work(struct fb_info *info)
@@ -437,10 +441,30 @@ static void fbcon_add_cursor_work(struct fb_info *info)
par->cur_blink_jiffies);
}
+/*
+ * Stop the cursor blink.
+ *
+ * Does not wait for a callback that is already running, so it is safe to
+ * call from any context.
+ *
+ * console_lock serializes this with the requeue in fb_flashcursor();
+ * otherwise a running callback could re-arm the work.
+ */
static void fbcon_del_cursor_work(struct fb_info *info)
{
struct fbcon_par *par = info->fbcon_par;
+ cancel_delayed_work(&par->cursor_work);
+}
+
+/*
+ * Stop the cursor blink and wait for a running callback to finish before
+ * @par is freed. May sleep, only for teardown paths.
+ */
+static void fbcon_del_cursor_work_sync(struct fb_info *info)
+{
+ struct fbcon_par *par = info->fbcon_par;
+
cancel_delayed_work_sync(&par->cursor_work);
}
@@ -765,7 +789,7 @@ static void fbcon_release(struct fb_info *info)
if (info->fbcon_par) {
struct fbcon_par *par = info->fbcon_par;
- fbcon_del_cursor_work(info);
+ fbcon_del_cursor_work_sync(info);
kfree(par->cursor_state.mask);
kfree(par->cursor_data);
kfree(par->cursor_src);
--
2.56.0
