* [PATCH 0/3] vt: make the console_lock ordering explicit for three variables
@ 2026-09-22 1:27 Jaidev Shastri via B4 Relay
2026-09-22 1:27 ` [PATCH 1/3] vt: order the fg_console switch against vt_console_print() Jaidev Shastri via B4 Relay
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Jaidev Shastri via B4 Relay @ 2026-09-22 1:27 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby; +Cc: linux-kernel, linux-serial, Jaidev Shastri
Three variables in vt.c are written on one path and read on another with
plain accesses on both sides: fg_console between redraw_screen() and
vt_console_print(), console_blanked between do_blank_screen() and
do_unblank_screen(), and the vc_cons[] slot between vc_deallocate() and
console_callback().
Every one of these pairs is serialised by console_lock today, so no behaviour
changes. The patches state the publication order in the accessors rather than
leaving it implicit in the lock, so the dependency is visible where the
variables are used.
Found with MBCheck, a static herd7-based memory consistency checker.
Compile-tested on arm64 with W=1, no new warnings.
---
Jaidev Shastri (3):
vt: order the fg_console switch against vt_console_print()
vt: order console_blanked between blanking and unblanking
vt: order the vc_cons[] clear against console_callback()
drivers/tty/vt/vt.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
---
base-commit: 93f51579e7df248780214094418f205253383cc5
change-id: 20260921-mb-vt-ca00f48031f0
Best regards,
--
Jaidev Shastri <jaidevshastri@vt.edu>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] vt: order the fg_console switch against vt_console_print()
2026-09-22 1:27 [PATCH 0/3] vt: make the console_lock ordering explicit for three variables Jaidev Shastri via B4 Relay
@ 2026-09-22 1:27 ` Jaidev Shastri via B4 Relay
2026-09-22 1:45 ` sashiko-bot
2026-09-23 12:50 ` Greg Kroah-Hartman
2026-09-22 1:27 ` [PATCH 2/3] vt: order console_blanked between blanking and unblanking Jaidev Shastri via B4 Relay
2026-09-22 1:27 ` [PATCH 3/3] vt: order the vc_cons[] clear against console_callback() Jaidev Shastri via B4 Relay
2 siblings, 2 replies; 8+ messages in thread
From: Jaidev Shastri via B4 Relay @ 2026-09-22 1:27 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby; +Cc: linux-kernel, linux-serial, Jaidev Shastri
From: Jaidev Shastri <jaidevshastri@vt.edu>
redraw_screen() switches fg_console with a plain store once the new
console's state is set up. vt_console_print(), the printk console
callback, indexes vc_cons[] with fg_console before it takes
printing_lock.
Store the new index with smp_store_release() and read it with
smp_load_acquire(), so that the printk path cannot reach the slot before
the console it denotes is complete.
Found with MBCheck, a static herd7-based memory consistency checker.
Signed-off-by: Jaidev Shastri <jaidevshastri@vt.edu>
---
drivers/tty/vt/vt.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 57edf3749..3abbd6cf9 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -973,7 +973,8 @@ void redraw_screen(struct vc_data *vc, int is_switch)
if (!con_is_visible(vc))
redraw = 1;
*vc->vc_display_fg = vc;
- fg_console = vc->vc_num;
+ /* Pairs with the smp_load_acquire() in vt_console_print(). */
+ smp_store_release(&fg_console, vc->vc_num);
hide_cursor(old_vc);
if (!con_is_visible(old_vc)) {
save_screen(old_vc);
@@ -3447,7 +3448,8 @@ int vt_kmsg_redirect(int new)
static void vt_console_print(struct console *co, const char *b, unsigned count)
{
- struct vc_data *vc = vc_cons[fg_console].d;
+ /* Pairs with the smp_store_release() in redraw_screen(). */
+ struct vc_data *vc = vc_cons[smp_load_acquire(&fg_console)].d;
unsigned char c;
static DEFINE_SPINLOCK(printing_lock);
const ushort *start;
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] vt: order console_blanked between blanking and unblanking
2026-09-22 1:27 [PATCH 0/3] vt: make the console_lock ordering explicit for three variables Jaidev Shastri via B4 Relay
2026-09-22 1:27 ` [PATCH 1/3] vt: order the fg_console switch against vt_console_print() Jaidev Shastri via B4 Relay
@ 2026-09-22 1:27 ` Jaidev Shastri via B4 Relay
2026-09-22 1:38 ` sashiko-bot
2026-09-22 1:27 ` [PATCH 3/3] vt: order the vc_cons[] clear against console_callback() Jaidev Shastri via B4 Relay
2 siblings, 1 reply; 8+ messages in thread
From: Jaidev Shastri via B4 Relay @ 2026-09-22 1:27 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby; +Cc: linux-kernel, linux-serial, Jaidev Shastri
From: Jaidev Shastri <jaidevshastri@vt.edu>
do_blank_screen() records the blanked console in console_blanked with a
plain store, after blank_state and the console state have been updated.
do_unblank_screen() tests console_blanked with a plain load before it
uses that state.
Store it with smp_store_release() and read it with smp_load_acquire().
Found with MBCheck, a static herd7-based memory consistency checker.
Signed-off-by: Jaidev Shastri <jaidevshastri@vt.edu>
---
drivers/tty/vt/vt.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 3abbd6cf9..726402167 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -4683,7 +4683,8 @@ void do_blank_screen(int entering_gfx)
hide_cursor(vc);
save_screen(vc);
vc->vc_sw->con_blank(vc, VESA_VSYNC_SUSPEND, 1);
- console_blanked = fg_console + 1;
+ /* Pairs with the smp_load_acquire() in do_unblank_screen(). */
+ smp_store_release(&console_blanked, fg_console + 1);
blank_state = blank_off;
set_origin(vc);
return;
@@ -4693,7 +4694,8 @@ void do_blank_screen(int entering_gfx)
/* don't blank graphics */
if (vc->vc_mode != KD_TEXT) {
- console_blanked = fg_console + 1;
+ /* Pairs with the smp_load_acquire() in do_unblank_screen(). */
+ smp_store_release(&console_blanked, fg_console + 1);
return;
}
@@ -4705,7 +4707,8 @@ void do_blank_screen(int entering_gfx)
/* In case we need to reset origin, blanking hook returns 1 */
i = vc->vc_sw->con_blank(vc, vesa_off_interval ? VESA_VSYNC_SUSPEND :
(vesa_blank_mode + 1), 0);
- console_blanked = fg_console + 1;
+ /* Pairs with the smp_load_acquire() in do_unblank_screen(). */
+ smp_store_release(&console_blanked, fg_console + 1);
if (i)
set_origin(vc);
@@ -4737,7 +4740,8 @@ void do_unblank_screen(int leaving_gfx)
WARN_CONSOLE_UNLOCKED();
ignore_poke = 0;
- if (!console_blanked)
+ /* Pairs with the smp_store_release() in do_blank_screen(). */
+ if (!smp_load_acquire(&console_blanked))
return;
if (!vc_cons_allocated(fg_console)) {
/* impossible */
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] vt: order the vc_cons[] clear against console_callback()
2026-09-22 1:27 [PATCH 0/3] vt: make the console_lock ordering explicit for three variables Jaidev Shastri via B4 Relay
2026-09-22 1:27 ` [PATCH 1/3] vt: order the fg_console switch against vt_console_print() Jaidev Shastri via B4 Relay
2026-09-22 1:27 ` [PATCH 2/3] vt: order console_blanked between blanking and unblanking Jaidev Shastri via B4 Relay
@ 2026-09-22 1:27 ` Jaidev Shastri via B4 Relay
2026-09-22 1:38 ` sashiko-bot
2 siblings, 1 reply; 8+ messages in thread
From: Jaidev Shastri via B4 Relay @ 2026-09-22 1:27 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby; +Cc: linux-kernel, linux-serial, Jaidev Shastri
From: Jaidev Shastri <jaidevshastri@vt.edu>
vc_deallocate() frees the console's screen buffer and unimap and then
clears vc_cons[currcons].d with a plain store. console_callback() reads
vc_cons[fg_console].d with a plain load.
Clear the slot with smp_store_release() and read it with
smp_load_acquire().
Found with MBCheck, a static herd7-based memory consistency checker.
Signed-off-by: Jaidev Shastri <jaidevshastri@vt.edu>
---
drivers/tty/vt/vt.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 726402167..66aa089bf 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -1369,7 +1369,8 @@ struct vc_data *vc_deallocate(unsigned int currcons)
put_pid(vc->vt_pid);
vc_uniscr_set(vc, NULL);
kfree(vc->vc_screenbuf);
- vc_cons[currcons].d = NULL;
+ /* Pairs with the smp_load_acquire() in console_callback(). */
+ smp_store_release(&vc_cons[currcons].d, NULL);
if (vc->vc_saved_screen != NULL) {
kfree(vc->vc_saved_screen);
vc->vc_saved_screen = NULL;
@@ -3368,7 +3369,8 @@ static void console_callback(struct work_struct *ignored)
poke_blanked_console();
}
if (scrollback_delta) {
- struct vc_data *vc = vc_cons[fg_console].d;
+ /* Pairs with the smp_store_release() in vc_deallocate(). */
+ struct vc_data *vc = smp_load_acquire(&vc_cons[fg_console].d);
clear_selection();
if (vc->vc_mode == KD_TEXT && vc->vc_sw->con_scrolldelta)
vc->vc_sw->con_scrolldelta(vc, scrollback_delta);
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] vt: order the vc_cons[] clear against console_callback()
2026-09-22 1:27 ` [PATCH 3/3] vt: order the vc_cons[] clear against console_callback() Jaidev Shastri via B4 Relay
@ 2026-09-22 1:38 ` sashiko-bot
0 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-09-22 1:38 UTC (permalink / raw)
To: Jaidev Shastri; +Cc: linux-serial
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] tty: vt: unnecessary and flawed lockless synchronization under console_lock
--
commit 49da38d5b190153cd048046d7ba8249893ec05f7
Author: Jaidev Shastri <jaidevshastri@vt.edu>
vt: order the vc_cons[] clear against console_callback()
This commit replaces plain stores and loads of vc_cons[].d with
smp_store_release() and smp_load_acquire() in vc_deallocate() and
console_callback(). It aims to resolve a static memory consistency checker
warning.
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 7264021672baa..66aa089bff43a 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -1369,7 +1369,8 @@ struct vc_data *vc_deallocate(unsigned int currcons)
> put_pid(vc->vt_pid);
> vc_uniscr_set(vc, NULL);
> kfree(vc->vc_screenbuf);
> - vc_cons[currcons].d = NULL;
> + /* Pairs with the smp_load_acquire() in console_callback(). */
> + smp_store_release(&vc_cons[currcons].d, NULL);
[Severity: Medium]
Are these memory barriers necessary? Both vc_deallocate() and
console_callback() execute strictly under console_lock. vc_deallocate()
asserts this via WARN_CONSOLE_UNLOCKED(), and console_callback() acquires
it via guard(console_lock)() at the top of the function.
Furthermore, does this unpublish the pointer too late for a lockless reader?
Since it is unpublished after freeing resources like vc_screenbuf, wouldn't
this create a use-after-free window for any hypothetical lockless reader?
> if (vc->vc_saved_screen != NULL) {
> kfree(vc->vc_saved_screen);
> vc->vc_saved_screen = NULL;
[ ... ]
> @@ -3368,7 +3369,8 @@ static void console_callback(struct work_struct *ignored)
> poke_blanked_console();
> }
> if (scrollback_delta) {
> - struct vc_data *vc = vc_cons[fg_console].d;
> + /* Pairs with the smp_store_release() in vc_deallocate(). */
> + struct vc_data *vc = smp_load_acquire(&vc_cons[fg_console].d);
[Severity: Medium]
Since console_lock is held here, is this acquire barrier needed?
Also, doesn't console_callback() still contain plain loads of this same
variable that bypass this acquire barrier? For example, earlier in the
function:
hide_cursor(vc_cons[fg_console].d);
And later in the function:
notify_update(vc_cons[fg_console].d);
This appears to leave the lockless synchronization incomplete if it were
actually needed.
> clear_selection();
> if (vc->vc_mode == KD_TEXT && vc->vc_sw->con_scrolldelta)
> vc->vc_sw->con_scrolldelta(vc, scrollback_delta);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260921-mb-vt-v1-0-71b7990c26c1@vt.edu?part=3
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] vt: order console_blanked between blanking and unblanking
2026-09-22 1:27 ` [PATCH 2/3] vt: order console_blanked between blanking and unblanking Jaidev Shastri via B4 Relay
@ 2026-09-22 1:38 ` sashiko-bot
0 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-09-22 1:38 UTC (permalink / raw)
To: Jaidev Shastri; +Cc: linux-serial
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] vt: redundant barriers for console_blanked under console_lock
- [Medium] vt: smp_store_release placed before protected state updates
--
commit e185177321eb3b6f0a4a6c836f482dfda649b14a
Author: Jaidev Shastri <jaidevshastri@vt.edu>
vt: order console_blanked between blanking and unblanking
This commit adds smp_store_release() and smp_load_acquire() around
console_blanked in do_blank_screen() and do_unblank_screen() to order its
updates against blank_state and the console state.
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 3abbd6cf9bd69..7264021672baa 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -4683,7 +4683,8 @@ void do_blank_screen(int entering_gfx)
> hide_cursor(vc);
> save_screen(vc);
> vc->vc_sw->con_blank(vc, VESA_VSYNC_SUSPEND, 1);
> - console_blanked = fg_console + 1;
> + /* Pairs with the smp_load_acquire() in do_unblank_screen(). */
> + smp_store_release(&console_blanked, fg_console + 1);
[Severity: Medium]
Are these barriers necessary for synchronization between do_blank_screen() and
do_unblank_screen()?
Looking at do_blank_screen(), it expects the console lock to be held:
might_sleep();
WARN_CONSOLE_UNLOCKED();
do_unblank_screen() also expects the console lock to be held:
WARN_CONSOLE_UNLOCKED();
Since both functions are fully serialized by the console lock, wouldn't these
barriers be redundant for synchronizing between them?
Additionally, the commit message states that do_blank_screen() stores the value
after blank_state and the console state have been updated. However, the release
store is placed before blank_state and set_origin(vc) are updated here:
smp_store_release(&console_blanked, fg_console + 1);
blank_state = blank_off;
set_origin(vc);
Does this fail to provide the memory ordering guaranteed by the commit message
for concurrent lockless readers?
> blank_state = blank_off;
> set_origin(vc);
> return;
[ ... ]
> @@ -4705,7 +4707,8 @@ void do_blank_screen(int entering_gfx)
> /* In case we need to reset origin, blanking hook returns 1 */
> i = vc->vc_sw->con_blank(vc, vesa_off_interval ? VESA_VSYNC_SUSPEND :
> (vesa_blank_mode + 1), 0);
> - console_blanked = fg_console + 1;
> + /* Pairs with the smp_load_acquire() in do_unblank_screen(). */
> + smp_store_release(&console_blanked, fg_console + 1);
[Severity: Medium]
Similar to the previous case, the release store is placed before the potential
call to set_origin(vc), leaving the console state update unordered:
smp_store_release(&console_blanked, fg_console + 1);
if (i)
set_origin(vc);
Could a concurrent reader acquire console_blanked but still read stale values
for the console origin?
> if (i)
> set_origin(vc);
>
[ ... ]
> @@ -4737,7 +4740,8 @@ void do_unblank_screen(int leaving_gfx)
> WARN_CONSOLE_UNLOCKED();
>
> ignore_poke = 0;
> - if (!console_blanked)
> + /* Pairs with the smp_store_release() in do_blank_screen(). */
> + if (!smp_load_acquire(&console_blanked))
[Severity: Medium]
If the intention was to protect true lockless readers such as tioclinux():
case TIOCL_BLANKEDSCREEN:
return console_blanked;
is the protection incomplete? console_blanked is still updated with a plain
store later in do_unblank_screen():
console_blanked = 0;
Would this plain store remain exposed to those lockless readers and leave the
data race intact?
> return;
> if (!vc_cons_allocated(fg_console)) {
> /* impossible */
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260921-mb-vt-v1-0-71b7990c26c1@vt.edu?part=2
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] vt: order the fg_console switch against vt_console_print()
2026-09-22 1:27 ` [PATCH 1/3] vt: order the fg_console switch against vt_console_print() Jaidev Shastri via B4 Relay
@ 2026-09-22 1:45 ` sashiko-bot
2026-09-23 12:50 ` Greg Kroah-Hartman
1 sibling, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-09-22 1:45 UTC (permalink / raw)
To: Jaidev Shastri; +Cc: linux-serial
> From: Jaidev Shastri <jaidevshastri@vt.edu>
>
> redraw_screen() switches fg_console with a plain store once the new
> console's state is set up. vt_console_print(), the printk console
> callback, indexes vc_cons[] with fg_console before it takes
> printing_lock.
>
> Store the new index with smp_store_release() and read it with
> smp_load_acquire(), so that the printk path cannot reach the slot before
> the console it denotes is complete.
>
> Found with MBCheck, a static herd7-based memory consistency checker.
>
> Signed-off-by: Jaidev Shastri <jaidevshastri@vt.edu>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260921-mb-vt-v1-0-71b7990c26c1@vt.edu?part=1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] vt: order the fg_console switch against vt_console_print()
2026-09-22 1:27 ` [PATCH 1/3] vt: order the fg_console switch against vt_console_print() Jaidev Shastri via B4 Relay
2026-09-22 1:45 ` sashiko-bot
@ 2026-09-23 12:50 ` Greg Kroah-Hartman
1 sibling, 0 replies; 8+ messages in thread
From: Greg Kroah-Hartman @ 2026-09-23 12:50 UTC (permalink / raw)
To: jaidevshastri; +Cc: Jiri Slaby, linux-kernel, linux-serial
On Mon, Sep 21, 2026 at 09:27:14PM -0400, Jaidev Shastri via B4 Relay wrote:
> From: Jaidev Shastri <jaidevshastri@vt.edu>
>
> redraw_screen() switches fg_console with a plain store once the new
> console's state is set up. vt_console_print(), the printk console
> callback, indexes vc_cons[] with fg_console before it takes
> printing_lock.
>
> Store the new index with smp_store_release() and read it with
> smp_load_acquire(), so that the printk path cannot reach the slot before
> the console it denotes is complete.
>
> Found with MBCheck, a static herd7-based memory consistency checker.
>
> Signed-off-by: Jaidev Shastri <jaidevshastri@vt.edu>
> ---
> drivers/tty/vt/vt.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 57edf3749..3abbd6cf9 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -973,7 +973,8 @@ void redraw_screen(struct vc_data *vc, int is_switch)
> if (!con_is_visible(vc))
> redraw = 1;
> *vc->vc_display_fg = vc;
> - fg_console = vc->vc_num;
> + /* Pairs with the smp_load_acquire() in vt_console_print(). */
> + smp_store_release(&fg_console, vc->vc_num);
Using these functions are almost always wrong. Fix things properly, do
not pepper these types of calls all over the kernel, that way lies
madness.
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-09-23 12:50 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-22 1:27 [PATCH 0/3] vt: make the console_lock ordering explicit for three variables Jaidev Shastri via B4 Relay
2026-09-22 1:27 ` [PATCH 1/3] vt: order the fg_console switch against vt_console_print() Jaidev Shastri via B4 Relay
2026-09-22 1:45 ` sashiko-bot
2026-09-23 12:50 ` Greg Kroah-Hartman
2026-09-22 1:27 ` [PATCH 2/3] vt: order console_blanked between blanking and unblanking Jaidev Shastri via B4 Relay
2026-09-22 1:38 ` sashiko-bot
2026-09-22 1:27 ` [PATCH 3/3] vt: order the vc_cons[] clear against console_callback() Jaidev Shastri via B4 Relay
2026-09-22 1:38 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox