[PATCH] speakup: handle serial IRQ request failure

[PATCH] speakup: handle serial IRQ request failure

[Runyu Xiao]

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@vger.kernel.org
Assisted-by: LLM Codex
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 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

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@vger.kernel.org
> Assisted-by: LLM Codex
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
> ---
>  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

[PATCH] speakup: handle serial IRQ request failure

[Runyu Xiao]

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.

Reproducer:

  Build an x86_64 kernel with CONFIG_ACCESSIBILITY=y,
  CONFIG_SPEAKUP=m, and CONFIG_SERIAL_8250=m. Disable ACPI, PNP, and
  SERIAL_8250_CONSOLE so that QEMU's COM1 remains available at 0x3f8 on
  IRQ4. Build an out-of-tree module that first claims IRQ4 with a
  non-shared handler and then registers a Speakup serial synthesizer for
  ttyS0. Boot the kernel in QEMU with its default COM1, for example:

    qemu-system-x86_64 -machine pc -m 1024 -smp 2 -no-acpi \
      -display none -monitor none -serial file:/dev/null \
      -kernel arch/x86/boot/bzImage -initrd test.cpio.gz \
      -append 'console=hvc0 rdinit=/init loglevel=7'

  In the guest, load the modules in this order:

    insmod speakup.ko
    insmod speakup_irq_failure_poc.ko

  The second module keeps the non-shared IRQ4 owner installed, so the
  Speakup request_irq(..., IRQF_SHARED, ...) fails with -EBUSY. On the
  unfixed kernel, the probe still returns success and the harness removes
  the published synthesizer, producing "Trying to free already-free IRQ
  4". On the fixed kernel, the probe returns an error and no unmatched
  free_irq() warning is emitted. The IRQ conflict is 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@vger.kernel.org
Assisted-by: LLM Codex
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 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