linux-kernel - [PATCH] usb: gadget: u_serial: fix null-ptr-deref in gs_start

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240926064910.17429-1-hubert.buczynski94@gmail.com>
Date: Thu, 26 Sep 2024 08:49:10 +0200
From: hbuczynski <hubert.buczynski94@...il.com>
To: balbi@...nel.org,
	gregkh@...uxfoundation.org,
	quic_prashk@...cinc.com
Cc: linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	"hubert.buczynski" <Hubert.Buczynski.ext@...g.de>
Subject: [PATCH] usb: gadget: u_serial: fix null-ptr-deref in gs_start_io

From: "hubert.buczynski" <Hubert.Buczynski.ext@...g.de>

The commit "5a444bea usb: gadget: u_serial: Set start_delayed during
suspend" caused invocation of the gs_start_io in the gserial_resume.
The gs_start_io doesn't check the ptr of the 'port.tty'. As a result, the
tty_wakeup function is passed on to the NULL ptr causing kernel panic.

There is a deterministic scenario leading to the kernel error:
1. Set the device in peripheral OTG mode.
2. Attach the USB cable to the host.
3. Do not take any action on the host side.
4. Send data to the host, for example:
$ echo "hello\n" > /dev/ttyGS0
5. Disconnect the USB cable.
6. Connect the USB cable and the kernel panic should be visible.

Fragment of the kernel panic log message:

Internal error: Oops: 5 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 0 PID: 0 Comm: swapper/0 Tainted: P O 5.15.166 #88
Hardware name: STM32 hDevice Tree Support)
PC is at tty_wakeup+0x8/0x5c
LR is at gs_start_io+0x90/0xdc
pc : [<c0623f74>]    lr : [<c083eeac>]    psr: 60010193
sp : c1001da0  ip : c32e6944  fp : 80000000
r10: c32e6934  r9 : c32e6928  r8 : c32e68ec
r7 : c32e68e0  r6 : c2be6c40  r5 : 00000000  r4 : 00000000
r3 : 00000000  r2 : 00000000  r1 : 60010193  r0 : 00000000
Flags: nZC»  IRQs off  FIQs on  Mode SVC_32  ISA ARM Segment none
Control: 10c5387d  Table: c3ac406a  DAC: 00000051
Register r0 information: NULL pointer
Register r1 information: non-paged memory
Register r2 information: NULL pointer
Register r3 information: NULL pointer
Register r4 information: NULL pointer
Register r5 information: NULL pointer
[<c0623f74>] (tty_wakeup) from [<c083eeac>] (gs_start_io+0x90/0xdc)
[<c083eeac>] (gs_start_io) from [<c083f0c0>](gserial_resume+0x6c/0xd4)
[<c083f0c0>] (gserial_resume) from [<c082a35c>] (composite_resume+0x70/0x10c)
[<c082a35c>] (composite_resume) from [<c082d668>] (configfs_composite_resume+0x54/0x64)
[<c082d668>] (configfs_composite_resume) from [<c07c26c4>] (dwc2_handle_wakeup_detected_intr+0x15c/0x2e8)
[<c07c26c4>] (dwc2_handle_wakeup_detected_intr) from [<c07c2c74>] (dwc2_handle_common_intr+0x424/0x630)
[<c07c2c74>] (dwc2_handle_common_intr) from [<c0190168>] (__handle_irq_event_percpu+0x50/0x250)
[<c0190168>] (__handle_irq_event_percpu) from [<c0190440>] (handle_irq_event+0x58/0xc4)
[<c0190440>] (handle_irq_event) from [<c0194f9c>] (handle_fasteoi_irq+0x9c/0x204)
[<c0194f9c>] (handle_fasteoi_irq) from [<c018fb2c>] (handle_domain_irq+0x58/0x74)
[<c018fb2c>] (handle_domain_irq) from [<c0101328>] (gic_handle_irq+0x7c/0x90)
[<c0101328>] (gic_handle_irq) from [<c0100b7c>] (__irq_svc+0x5c/0x90)

If the device sends data and does not receive msg from the host then the
field port->read_started contains a positive value. After disconnecting
the cable, gserial_suspend() is invoked and the port->start_delayed is set
to true. Connecting the cable again causes invocation of the
gserial_resume().
The callstack after connecting the cable looks like the following:
gserial_resume()
  --> gs_start_io()
    --> tty_wakeup() - with NULL argument

Fixes: 5a444bea37e2 ("usb: gadget: u_serial: Set start_delayed during suspend")

Signed-off-by: hubert.buczynski <Hubert.Buczynski.ext@...g.de>
---
 drivers/usb/gadget/function/u_serial.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
index 5111fcc0cac3..384f219fe01d 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -564,7 +564,7 @@ static int gs_start_io(struct gs_port *port)
 	port->n_read = 0;
 	started = gs_start_rx(port);
 
-	if (started) {
+	if (started && port->port.tty) {
 		gs_start_tx(port);
 		/* Unblock any pending writes into our circular buffer, in case
 		 * we didn't in gs_start_tx() */
-- 
2.25.1