* [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* 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
* [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* 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
* [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