From: Samuel Thibault <samuel.thibault@ens-lyon.org>
To: Hongyu Xie <xiehongyu1@kylinos.cn>
Cc: speakup@linux-speakup.org, linux-kernel@vger.kernel.org,
gregkh@linuxfoundation.org, chris@the-brannons.com,
kirk@reisers.ca, w.d.hubbs@gmail.com, stable@vger.kernel.org
Subject: Re: [PATCH 1/4] speakup: Stop catch_up when the synth is not alive
Date: Sun, 18 Oct 2026 20:24:07 +0200 [thread overview]
Message-ID: <atUOx28JlfcIUqHH@end> (raw)
In-Reply-To: <20261014071713.321914-2-xiehongyu1@kylinos.cn>
Hongyu Xie, le mer. 14 oct. 2026 15:17:10 +0800, a ecrit:
> When the synth output cannot make progress, the catch-up loop in
> _spk_do_catch_up() (and in its local copies in the serial synth
> drivers) can spin forever: nothing tells it that the synth is dead.
> This happens
>
> - when the tty write keeps returning 0 (no room) because nothing
> consumes the data, e.g. a serial port with no device attached that
> keeps flow control asserted, or
> - when an I/O error deactivates the synth: synth_out() then keeps
> returning 0 and the loop never notices that synth->alive became 0.
>
> While speakup_thread() holds spk_mutex across the whole loop, any
> synth_add(), synth_remove() or synth_init() then blocks forever,
> which gets reported as a hung task on module unload:
>
> INFO: task modprobe:2999919 blocked for more than 120 seconds.
> mutex_lock+0x54/0x68
> synth_remove+0x28/0xb8 [speakup]
> INFO: task modprobe:2999919 is blocked on a mutex likely owned
> by task speakup:
> schedule_timeout+...
> _spk_do_catch_up+...
> speakup_thread+...
>
> Check synth->alive in the loops as well, so that catch_up() stops as
> soon as the synth is dead. synth_release() writes it under
> speakup_info.spinlock, while the I/O error paths write it without the
> lock; that is fine, since the loops only need to eventually observe
> the change.
>
> On its own this fixes the I/O error case, where the synth is already
> deactivated when the loop retries. In the no-room case nothing
> clears synth->alive, and speakup_thread() still holds spk_mutex
> across the loop, so nothing can get in to deactivate the synth; the
> follow-up patch completes the fix by clearing synth->alive from
> synth_release() and by dropping spk_mutex from the catch_up() call.
>
> Fixes: 2067fd92d75b6 ("staging/speakup: Move out of staging")
> Cc: stable@vger.kernel.org
> Cc: Samuel Thibault <samuel.thibault@ens-lyon.org>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Thanks!
Samuel
> Signed-off-by: Hongyu Xie <xiehongyu1@kylinos.cn>
> ---
> drivers/accessibility/speakup/speakup_acntpc.c | 4 ++++
> drivers/accessibility/speakup/speakup_apollo.c | 4 ++++
> drivers/accessibility/speakup/speakup_decext.c | 4 ++++
> drivers/accessibility/speakup/speakup_decpc.c | 4 ++++
> drivers/accessibility/speakup/speakup_dectlk.c | 4 ++++
> drivers/accessibility/speakup/speakup_dtlk.c | 4 ++++
> drivers/accessibility/speakup/speakup_keypc.c | 4 ++++
> drivers/accessibility/speakup/synth.c | 4 ++++
> 8 files changed, 32 insertions(+)
>
> diff --git a/drivers/accessibility/speakup/speakup_acntpc.c b/drivers/accessibility/speakup/speakup_acntpc.c
> index a27e6bbf05da3..fe19ba3f00053 100644
> --- a/drivers/accessibility/speakup/speakup_acntpc.c
> +++ b/drivers/accessibility/speakup/speakup_acntpc.c
> @@ -192,6 +192,10 @@ static void do_catch_up(struct spk_synth *synth)
> jiff_max = jiffies + jiffy_delta_val;
> while (!kthread_should_stop()) {
> spin_lock_irqsave(&speakup_info.spinlock, flags);
> + if (!synth->alive) {
> + spin_unlock_irqrestore(&speakup_info.spinlock, flags);
> + break;
> + }
> if (speakup_info.flushing) {
> speakup_info.flushing = 0;
> spin_unlock_irqrestore(&speakup_info.spinlock, flags);
> diff --git a/drivers/accessibility/speakup/speakup_apollo.c b/drivers/accessibility/speakup/speakup_apollo.c
> index d2fbb3f572219..b62696e050fc0 100644
> --- a/drivers/accessibility/speakup/speakup_apollo.c
> +++ b/drivers/accessibility/speakup/speakup_apollo.c
> @@ -157,6 +157,10 @@ static void do_catch_up(struct spk_synth *synth)
>
> while (!kthread_should_stop()) {
> spin_lock_irqsave(&speakup_info.spinlock, flags);
> + if (!synth->alive) {
> + spin_unlock_irqrestore(&speakup_info.spinlock, flags);
> + break;
> + }
> jiffy_delta_val = jiffy_delta->u.n.value;
> full_time_val = full_time->u.n.value;
> delay_time_val = delay_time->u.n.value;
> diff --git a/drivers/accessibility/speakup/speakup_decext.c b/drivers/accessibility/speakup/speakup_decext.c
> index 271bcf279bf9b..e6be5a99670bd 100644
> --- a/drivers/accessibility/speakup/speakup_decext.c
> +++ b/drivers/accessibility/speakup/speakup_decext.c
> @@ -171,6 +171,10 @@ static void do_catch_up(struct spk_synth *synth)
>
> while (!kthread_should_stop()) {
> spin_lock_irqsave(&speakup_info.spinlock, flags);
> + if (!synth->alive) {
> + spin_unlock_irqrestore(&speakup_info.spinlock, flags);
> + break;
> + }
> if (speakup_info.flushing) {
> speakup_info.flushing = 0;
> spin_unlock_irqrestore(&speakup_info.spinlock, flags);
> diff --git a/drivers/accessibility/speakup/speakup_decpc.c b/drivers/accessibility/speakup/speakup_decpc.c
> index 1bf36d1e54773..cfafd94759639 100644
> --- a/drivers/accessibility/speakup/speakup_decpc.c
> +++ b/drivers/accessibility/speakup/speakup_decpc.c
> @@ -391,6 +391,10 @@ static void do_catch_up(struct spk_synth *synth)
>
> while (!kthread_should_stop()) {
> spin_lock_irqsave(&speakup_info.spinlock, flags);
> + if (!synth->alive) {
> + spin_unlock_irqrestore(&speakup_info.spinlock, flags);
> + break;
> + }
> if (speakup_info.flushing) {
> speakup_info.flushing = 0;
> spin_unlock_irqrestore(&speakup_info.spinlock, flags);
> diff --git a/drivers/accessibility/speakup/speakup_dectlk.c b/drivers/accessibility/speakup/speakup_dectlk.c
> index 56334405d865c..d9247c53d3275 100644
> --- a/drivers/accessibility/speakup/speakup_dectlk.c
> +++ b/drivers/accessibility/speakup/speakup_dectlk.c
> @@ -246,6 +246,10 @@ static void do_catch_up(struct spk_synth *synth)
> spin_unlock_irqrestore(&flush_lock, flags);
>
> spin_lock_irqsave(&speakup_info.spinlock, flags);
> + if (!synth->alive) {
> + spin_unlock_irqrestore(&speakup_info.spinlock, flags);
> + break;
> + }
> if (speakup_info.flushing) {
> speakup_info.flushing = 0;
> spin_unlock_irqrestore(&speakup_info.spinlock, flags);
> diff --git a/drivers/accessibility/speakup/speakup_dtlk.c b/drivers/accessibility/speakup/speakup_dtlk.c
> index fa826568937bf..73cd39e781b86 100644
> --- a/drivers/accessibility/speakup/speakup_dtlk.c
> +++ b/drivers/accessibility/speakup/speakup_dtlk.c
> @@ -206,6 +206,10 @@ static void do_catch_up(struct spk_synth *synth)
> jiff_max = jiffies + jiffy_delta_val;
> while (!kthread_should_stop()) {
> spin_lock_irqsave(&speakup_info.spinlock, flags);
> + if (!synth->alive) {
> + spin_unlock_irqrestore(&speakup_info.spinlock, flags);
> + break;
> + }
> if (speakup_info.flushing) {
> speakup_info.flushing = 0;
> spin_unlock_irqrestore(&speakup_info.spinlock, flags);
> diff --git a/drivers/accessibility/speakup/speakup_keypc.c b/drivers/accessibility/speakup/speakup_keypc.c
> index 9356f63795603..e229009107f75 100644
> --- a/drivers/accessibility/speakup/speakup_keypc.c
> +++ b/drivers/accessibility/speakup/speakup_keypc.c
> @@ -193,6 +193,10 @@ static void do_catch_up(struct spk_synth *synth)
> jiff_max = jiffies + jiffy_delta_val;
> while (!kthread_should_stop()) {
> spin_lock_irqsave(&speakup_info.spinlock, flags);
> + if (!synth->alive) {
> + spin_unlock_irqrestore(&speakup_info.spinlock, flags);
> + break;
> + }
> if (speakup_info.flushing) {
> speakup_info.flushing = 0;
> spin_unlock_irqrestore(&speakup_info.spinlock, flags);
> diff --git a/drivers/accessibility/speakup/synth.c b/drivers/accessibility/speakup/synth.c
> index 11cba1fd8715d..6a49ff475c16a 100644
> --- a/drivers/accessibility/speakup/synth.c
> +++ b/drivers/accessibility/speakup/synth.c
> @@ -75,6 +75,10 @@ static void _spk_do_catch_up(struct spk_synth *synth, int unicode)
> jiff_max = jiffies + jiffy_delta_val;
> while (!kthread_should_stop()) {
> spin_lock_irqsave(&speakup_info.spinlock, flags);
> + if (!synth->alive) {
> + spin_unlock_irqrestore(&speakup_info.spinlock, flags);
> + break;
> + }
> if (speakup_info.flushing) {
> speakup_info.flushing = 0;
> spin_unlock_irqrestore(&speakup_info.spinlock, flags);
> --
> 2.25.1
>
--
Samuel
<D> m'enfin, le 5 juillet, le mec vient visiter le labo...
* D a marque d'une croix rouge le 5 juillet sur son agenda
<y> niarc niarc niarc
<D> cet homme va souffrir
<B> c'est donc le 5 juillet qu'il meurt d'un accident de la route écrasé par un truck muni d'un pare buffle
-+- #ens-mim - repaire de terroristes -+-
next prev parent reply other threads:[~2026-10-18 18:24 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-14 7:17 [PATCH 0/4] speakup: fix hang and use-after-free around synth removal Hongyu Xie
2026-10-14 7:17 ` [PATCH 1/4] speakup: Stop catch_up when the synth is not alive Hongyu Xie
2026-10-18 18:24 ` Samuel Thibault [this message]
2026-10-14 7:17 ` [PATCH 2/4] speakup: Do not hold spk_mutex across catch_up() Hongyu Xie
2026-10-18 18:26 ` Samuel Thibault
2026-10-14 7:17 ` [PATCH 3/4] speakup: Guard late sysfs operations on speakup_kobj Hongyu Xie
2026-10-18 18:27 ` Samuel Thibault
2026-10-14 7:17 ` [PATCH 4/4] speakup: Serialize synth clearing against locked readers Hongyu Xie
2026-10-18 19:40 ` Samuel Thibault
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=atUOx28JlfcIUqHH@end \
--to=samuel.thibault@ens-lyon.org \
--cc=chris@the-brannons.com \
--cc=gregkh@linuxfoundation.org \
--cc=kirk@reisers.ca \
--cc=linux-kernel@vger.kernel.org \
--cc=speakup@linux-speakup.org \
--cc=stable@vger.kernel.org \
--cc=w.d.hubbs@gmail.com \
--cc=xiehongyu1@kylinos.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®