On 26-09-24 12:19 pm, hbuczynski wrote:
> From: "hubert.buczynski" <Hubert.Buczynski.ext@xxxxxxx>
>
> 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@xxxxxxx>
> ---
> 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() */
Commit ffd603f21423 ("usb: gadget: u_serial: Add null pointer check in
gs_start_io") fixed this issue. Please try adding it into your builds.
Regards,
Prashanth K
