Re: [PATCH 1/4] speakup: Stop catch_up when the synth is not alive

[Samuel Thibault]

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@xxxxxxxxxxxxxxx
> Cc: Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>

Reviewed-by: Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>

Thanks!

Samuel

> Signed-off-by: Hongyu Xie <xiehongyu1@xxxxxxxxxx>
> ---
>  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 -+-