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 callback that is already
running. Since fb_flashcursor() re-arms itself, a callback that has
already been dequeued by process_one_work() can queue the work again
after fbcon_del_cursor_work() returns. Add a cursor_blink_on flag to
struct fbcon_par: fbcon_add_cursor_work() sets it, fbcon_del_cursor_work()
clears it, and fb_flashcursor() checks it after taking console_lock and
returns without drawing or re-arming once blinking has been stopped.
Keep the requeue inside the console_lock section so that the flag check
and the requeue are not split by a concurrent stop.
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]>
---
Changes in v3:
- Add cursor_blink_on to struct fbcon_par so a callback that has already
been dequeued by process_one_work() cannot re-arm the work after
fbcon_del_cursor_work() has stopped blinking.
- Keep the requeue inside console_lock so the flag check and the requeue
are atomic with the stop.
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 | 41 +++++++++++++++++++++++++++-----
drivers/video/fbdev/core/fbcon.h | 1 +
2 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 94abbb5ac766..0847dcf18416 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -410,7 +410,8 @@ static void fb_flashcursor(struct work_struct *work)
if (par->currcon != -1)
vc = vc_cons[par->currcon].d;
- if (!vc || !con_is_visible(vc) ||
+ if (!par->cursor_blink_on ||
+ !vc || !con_is_visible(vc) ||
fbcon_info_from_console(vc->vc_num) != info ||
vc->vc_deccm != 1) {
console_unlock();
@@ -422,25 +423,53 @@ 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)
{
struct fbcon_par *par = info->fbcon_par;
- if (fbcon_cursor_blink)
- queue_delayed_work(system_power_efficient_wq, &par->cursor_work,
- par->cur_blink_jiffies);
+ if (!fbcon_cursor_blink)
+ return;
+
+ par->cursor_blink_on = true;
+ queue_delayed_work(system_power_efficient_wq, &par->cursor_work,
+ 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;
+ par->cursor_blink_on = false;
+ 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 +794,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);
diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h
index d93a89877d04..6a2f7f6e5a8c 100644
--- a/drivers/video/fbdev/core/fbcon.h
+++ b/drivers/video/fbdev/core/fbcon.h
@@ -97,6 +97,7 @@ struct fbcon_par {
int blank_state;
int graphics;
bool initialized;
+ bool cursor_blink_on;
int rotate;
char *cursor_data;
#ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION
--
2.56.0
