[PATCH v3 4/4] speakup: Serialize synth clearing against locked readers
From: Hongyu Xie
Date: Mon Nov 02 2026 - 20:15:44 EST
speakup_key() checks the global synth before taking
speakup_info.spinlock, but do_spkup() and its handlers dereference
synth inside that lock: speech_kill() calls synth->is_alive(), and
the flush path reaches synth_start() reading synth->alive.
synth_release() clears synth under spk_mutex, which does not
serialize against the key path, so a speakup key pressed while the
synth is being removed can pass the unlocked check and oops on the
stale pointer:
CPU0 (speakup key) CPU1 (rmmod -> synth_release)
------------------ ----------------------------
speakup_key():
if (!synth) mutex_lock(&spk_mutex)
return /* passes */ synth_release():
spin_lock(&speakup_info. ... drain in-flight catch_up() ...
spinlock) synth->release(synth)
do_spkup():
speech_kill():
synth->is_alive()
/* synth whose release() already tore down its I/O */
Close the window on both sides: clear synth under
speakup_info.spinlock in synth_release() and in the do_synth_init()
probe-failure path, and recheck it after acquiring the lock in every
reader that dereferences it there - do_spkup() at entry, the console
notifier paths after their trylock, do_handle_shift(),
do_handle_spec(), the speakup_key() special-handler dispatch,
synth_direct_store(), and the /dev/synth and /dev/synthu write
paths. The unlocked checks that used to gate these paths only
duplicated the in-lock recheck, with a window a clear could still
slip through, and are dropped; the only behavior difference is that
an empty write to /dev/synth or /dev/synthu with no synth loaded now
returns 0 instead of -ENODEV, which is harmless. The speakup thread
keeps its existing fence: synth_release() drains the catch-up count
before clearing.
Two sites of this shape remain, preexisting. The cursor_done()
timer callback dereferences synth inside the lock with no synth
check at all; closing it needs the cursor_timer lifetime handled
against synth teardown, which is beyond this series. The serial
read buffer handler (synth_readbuf_handler()) has the same shape but
cannot run in this tree: the only registration chain goes through
spk_serial_synth_probe(), which has no callers, and any
registration from outside the tree is still serialized by the
free_irq() in the serial synth release path, which runs before the
in-lock clear.
Fixes: 2067fd92d75b6 ("staging/speakup: Move out of staging")
Suggested-by: Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
Cc: Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>
Reviewed-by: Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>
Signed-off-by: Hongyu Xie <xiehongyu1@xxxxxxxxxx>
---
drivers/accessibility/speakup/devsynth.c | 13 ++++++----
drivers/accessibility/speakup/kobjects.c | 8 +++---
drivers/accessibility/speakup/main.c | 31 ++++++++++++++++++------
drivers/accessibility/speakup/synth.c | 4 +++
4 files changed, 41 insertions(+), 15 deletions(-)
diff --git a/drivers/accessibility/speakup/devsynth.c b/drivers/accessibility/speakup/devsynth.c
index e3d909bd0480..5a32d3bbf7b8 100644
--- a/drivers/accessibility/speakup/devsynth.c
+++ b/drivers/accessibility/speakup/devsynth.c
@@ -20,8 +20,6 @@ static ssize_t speakup_file_write(struct file *fp, const char __user *buffer,
unsigned long flags;
u_char buf[256];
- if (!synth)
- return -ENODEV;
while (count > 0) {
bytes = min(count, sizeof(buf));
if (copy_from_user(buf, ptr, bytes))
@@ -29,6 +27,10 @@ static ssize_t speakup_file_write(struct file *fp, const char __user *buffer,
count -= bytes;
ptr += bytes;
spin_lock_irqsave(&speakup_info.spinlock, flags);
+ if (!synth) {
+ spin_unlock_irqrestore(&speakup_info.spinlock, flags);
+ return -ENODEV;
+ }
synth_write(buf, bytes);
spin_unlock_irqrestore(&speakup_info.spinlock, flags);
}
@@ -47,9 +49,6 @@ static ssize_t speakup_file_writeu(struct file *fp, const char __user *buffer,
u16 ubuf[256];
size_t in, out;
- if (!synth)
- return -ENODEV;
-
want = 1;
while (count >= want) {
/* Copy some UTF-8 piece from userland */
@@ -84,6 +83,10 @@ static ssize_t speakup_file_writeu(struct file *fp, const char __user *buffer,
/* And speak this up */
if (out) {
spin_lock_irqsave(&speakup_info.spinlock, flags);
+ if (!synth) {
+ spin_unlock_irqrestore(&speakup_info.spinlock, flags);
+ return -ENODEV;
+ }
for (in = 0; in < out; in++)
synth_buffer_add(ubuf[in]);
synth_start();
diff --git a/drivers/accessibility/speakup/kobjects.c b/drivers/accessibility/speakup/kobjects.c
index e1860f8d9d42..45a8e9fbd8de 100644
--- a/drivers/accessibility/speakup/kobjects.c
+++ b/drivers/accessibility/speakup/kobjects.c
@@ -416,9 +416,6 @@ static ssize_t synth_direct_store(struct kobject *kobj,
char *unescaped;
unsigned long flags;
- if (!synth)
- return -EPERM;
-
unescaped = kstrdup(buf, GFP_KERNEL);
if (!unescaped)
return -ENOMEM;
@@ -426,6 +423,11 @@ static ssize_t synth_direct_store(struct kobject *kobj,
string_unescape_any_inplace(unescaped);
spin_lock_irqsave(&speakup_info.spinlock, flags);
+ if (!synth) {
+ spin_unlock_irqrestore(&speakup_info.spinlock, flags);
+ kfree(unescaped);
+ return -EPERM;
+ }
synth_write(unescaped, strlen(unescaped));
spin_unlock_irqrestore(&speakup_info.spinlock, flags);
diff --git a/drivers/accessibility/speakup/main.c b/drivers/accessibility/speakup/main.c
index e9b7c2761f6f..60be4d4df363 100644
--- a/drivers/accessibility/speakup/main.c
+++ b/drivers/accessibility/speakup/main.c
@@ -1166,9 +1166,13 @@ static void do_handle_shift(struct vc_data *vc, u_char value, char up_flag)
{
unsigned long flags;
- if (!synth || up_flag || spk_killed)
+ if (up_flag || spk_killed)
return;
spin_lock_irqsave(&speakup_info.spinlock, flags);
+ if (!synth) {
+ spin_unlock_irqrestore(&speakup_info.spinlock, flags);
+ return;
+ }
if (cursor_track == read_all_mode) {
switch (value) {
case KVAL(K_SHIFT):
@@ -1763,11 +1767,15 @@ static void speakup_con_write(struct vc_data *vc, u16 *str, int len)
{
unsigned long flags;
- if ((vc->vc_num != fg_console) || spk_shut_up || !synth)
+ if ((vc->vc_num != fg_console) || spk_shut_up)
return;
if (!spin_trylock_irqsave(&speakup_info.spinlock, flags))
/* Speakup output, discard */
return;
+ if (!synth) {
+ spin_unlock_irqrestore(&speakup_info.spinlock, flags);
+ return;
+ }
if (spk_bell_pos && spk_keydown && (vc->state.x == spk_bell_pos - 1))
bleep(3);
if ((is_cursor) || (cursor_track == read_all_mode)) {
@@ -1792,11 +1800,15 @@ static void speakup_con_update(struct vc_data *vc)
{
unsigned long flags;
- if (!speakup_console[vc->vc_num] || spk_parked || !synth)
+ if (!speakup_console[vc->vc_num] || spk_parked)
return;
if (!spin_trylock_irqsave(&speakup_info.spinlock, flags))
/* Speakup output, discard */
return;
+ if (!synth) {
+ spin_unlock_irqrestore(&speakup_info.spinlock, flags);
+ return;
+ }
speakup_date(vc);
if (vc->vc_mode == KD_GRAPHICS && !spk_paused && spk_str_pause[0]) {
synth_printf("%s", spk_str_pause);
@@ -1811,9 +1823,13 @@ static void do_handle_spec(struct vc_data *vc, u_char value, char up_flag)
int on_off = 2;
char *label;
- if (!synth || up_flag || spk_killed)
+ if (up_flag || spk_killed)
return;
spin_lock_irqsave(&speakup_info.spinlock, flags);
+ if (!synth) {
+ spin_unlock_irqrestore(&speakup_info.spinlock, flags);
+ return;
+ }
spk_shut_up &= 0xfe;
if (spk_no_intr)
spk_do_flush();
@@ -2087,6 +2103,8 @@ static spkup_hand spkup_handler[] = {
static void do_spkup(struct vc_data *vc, u_char value)
{
+ if (!synth)
+ return;
if (spk_killed && value != SPEECH_KILL)
return;
spk_keydown = 0;
@@ -2115,10 +2133,9 @@ speakup_key(struct vc_data *vc, int shift_state, int keycode, u16 keysym,
u_char shift_info, offset;
int ret = 0;
- if (!synth)
- return 0;
-
spin_lock_irqsave(&speakup_info.spinlock, flags);
+ if (!synth)
+ goto out;
tty = vc->port.tty;
if (type >= 0xf0)
type -= 0xf0;
diff --git a/drivers/accessibility/speakup/synth.c b/drivers/accessibility/speakup/synth.c
index 1af80eb116af..aef482f8b14e 100644
--- a/drivers/accessibility/speakup/synth.c
+++ b/drivers/accessibility/speakup/synth.c
@@ -519,7 +519,9 @@ static int do_synth_init(struct spk_synth *in_synth)
pr_warn("synth probe\n");
if (synth->probe(synth) < 0) {
pr_warn("%s: device probe failed\n", in_synth->name);
+ spin_lock_irqsave(&speakup_info.spinlock, flags);
synth = NULL;
+ spin_unlock_irqrestore(&speakup_info.spinlock, flags);
return -ENODEV;
}
synth_time_vars[0].u.n.value =
@@ -585,7 +587,9 @@ void synth_release(void)
for (var = synth->vars; var->var_id != MAXVARS; var++)
speakup_unregister_var(var->var_id);
synth->release(synth);
+ spin_lock_irqsave(&speakup_info.spinlock, flags);
synth = NULL;
+ spin_unlock_irqrestore(&speakup_info.spinlock, flags);
}
/* called by: all_driver_init() */
--
2.32.0