[PATCH] speakup: handle serial IRQ request failure

Runyu Xiao posted 1 patch 3 days, 3 hours ago
drivers/accessibility/speakup/serialio.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
[PATCH] speakup: handle serial IRQ request failure
Posted by Runyu Xiao 3 days, 3 hours ago
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