Re: [PATCH] speakup: handle serial IRQ request failure

[Samuel Thibault]

Hello,

Actually, this is old dead code. I'll send a patch to request removing
it.

Thanks for the report anyway,
Samuel

Runyu Xiao, le lun. 21 sept. 2026 15:50:35 +0800, a ecrit:
> start_serial_interrupt() currently logs a failed request_irq() but
> continues enabling UART interrupts and reports a successful serial
> synthesizer probe. A later release then calls free_irq() without a
> matching registration.
> 
> Return the request error to spk_serial_init() and unwind the I/O region
> and serial state when registration fails. This keeps the UART disabled
> and prevents teardown from freeing an IRQ that was never requested.
> 
> A QEMU x86_64 test used a non-shared handler to occupy COM1 IRQ4 before
> the Speakup serial path requested the same IRQ with IRQF_SHARED. On the
> unfixed kernel, the probe continued successfully and release emitted
> "Trying to free already-free IRQ 4". On the fixed kernel, probe failed
> and no unmatched free_irq() warning was emitted. The IRQ conflict was
> deliberately injected to exercise the error path and is not expected
> during normal operation.
> 
> Fixes: c6e3fd22cd53 ("Staging: add speakup to the staging directory")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: LLM Codex
> Signed-off-by: Runyu Xiao <runyu.xiao@xxxxxxxxxx>
> ---
>  drivers/accessibility/speakup/serialio.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/accessibility/speakup/serialio.c b/drivers/accessibility/speakup/serialio.c
> index 3418ea31d..56efc76a7 100644
> --- a/drivers/accessibility/speakup/serialio.c
> +++ b/drivers/accessibility/speakup/serialio.c
> @@ -17,7 +17,7 @@
>  #define SERIAL_PORT_DFNS
>  #endif
>  
> -static void start_serial_interrupt(int irq);
> +static int start_serial_interrupt(int irq);
>  
>  static const struct old_serial_port rs_table[] = {
>  	SERIAL_PORT_DFNS
> @@ -106,7 +106,13 @@ const struct old_serial_port *spk_serial_init(int index)
>  	speakup_info.port_tts = ser->port;
>  	serstate = ser;
>  
> -	start_serial_interrupt(ser->irq);
> +	err = start_serial_interrupt(ser->irq);
> +	if (err) {
> +		synth_release_region(ser->port, 8);
> +		speakup_info.port_tts = 0;
> +		serstate = NULL;
> +		return NULL;
> +	}
>  
>  	return ser;
>  }
> @@ -125,18 +131,20 @@ static irqreturn_t synth_readbuf_handler(int irq, void *dev_id)
>  	return IRQ_HANDLED;
>  }
>  
> -static void start_serial_interrupt(int irq)
> +static int start_serial_interrupt(int irq)
>  {
>  	int rv;
>  
>  	if (!synth->read_buff_add)
> -		return;
> +		return 0;
>  
>  	rv = request_irq(irq, synth_readbuf_handler, IRQF_SHARED,
>  			 "serial", (void *)synth_readbuf_handler);
>  
> -	if (rv)
> +	if (rv) {
>  		pr_err("Unable to request Speakup serial I R Q\n");
> +		return rv;
> +	}
>  	/* Set MCR */
>  	outb(UART_MCR_DTR | UART_MCR_RTS | UART_MCR_OUT2,
>  	     speakup_info.port_tts + UART_MCR);
> @@ -148,6 +156,7 @@ static void start_serial_interrupt(int irq)
>  	inb(speakup_info.port_tts + UART_IIR);
>  	inb(speakup_info.port_tts + UART_MSR);
>  	outb(1, speakup_info.port_tts + UART_FCR);	/* Turn FIFO On */
> +	return 0;
>  }
>  
>  static void spk_serial_send_xchar(struct spk_synth *synth, char ch)
> -- 
> 2.34.1
> 

-- 
Samuel
c> ah (on trouve fluide glacial sur le net, ou il faut aller dans le monde reel ?)
s> dans le monde reel
c> zut