From c78828e3832d88fb82e41f3d972ac6850c93fdfc Mon Sep 17 00:00:00 2001 From: Kuen-Han Tsai Date: Tue, 16 Jan 2024 22:16:17 +0800 Subject: [PATCH] FROMLIST: usb: gadget: u_serial: Add null pointer checks after RX/TX submission Commit ffd603f21423 ("usb: gadget: u_serial: Add null pointer check in gs_start_io") adds null pointer checks to gs_start_io(), but it doesn't fully fix the potential null pointer dereference issue. While gserial_connect() calls gs_start_io() with port_lock held, gs_start_rx() and gs_start_tx() release the lock during endpoint request submission. This creates a window where gs_close() could set port->port_tty to NULL, leading to a dereference when the lock is reacquired. This patch adds a null pointer check for port->port_tty after RX/TX submission, and removes the initial null pointer check in gs_start_io() since the caller must hold port_lock and guarantee non-null values for port_usb and port_tty. Fixes: ffd603f21423 ("usb: gadget: u_serial: Add null pointer check in gs_start_io") Cc: stable@vger.kernel.org Signed-off-by: Kuen-Han Tsai Bug: 283247551 Link: https://lore.kernel.org/lkml/20240116141801.396398-1-khtsai@google.com/ Change-Id: Ib850c7d313194074941576a7fdd3a9f58486ad78 Signed-off-by: Kuen-Han Tsai --- drivers/usb/gadget/function/u_serial.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c index b8854e8a771f..e466ac1cd9cb 100644 --- a/drivers/usb/gadget/function/u_serial.c +++ b/drivers/usb/gadget/function/u_serial.c @@ -538,20 +538,16 @@ static int gs_alloc_requests(struct usb_ep *ep, struct list_head *head, static int gs_start_io(struct gs_port *port) { struct list_head *head = &port->read_pool; - struct usb_ep *ep; + struct usb_ep *ep = port->port_usb->out; int status; unsigned started; - if (!port->port_usb || !port->port.tty) - return -EIO; - /* Allocate RX and TX I/O buffers. We can't easily do this much * earlier (with GFP_KERNEL) because the requests are coupled to * endpoints, as are the packet sizes we'll be using. Different * configurations may use different endpoints with a given port; * and high speed vs full speed changes packet sizes too. */ - ep = port->port_usb->out; status = gs_alloc_requests(ep, head, gs_read_complete, &port->read_allocated); if (status) @@ -568,12 +564,22 @@ static int gs_start_io(struct gs_port *port) port->n_read = 0; started = gs_start_rx(port); + /* + * The TTY may be set to NULL by gs_close() after gs_start_rx() or + * gs_start_tx() release locks for endpoint request submission. + */ + if (!port->port.tty) + goto out; + if (started) { gs_start_tx(port); /* Unblock any pending writes into our circular buffer, in case * we didn't in gs_start_tx() */ + if (!port->port.tty) + goto out; tty_wakeup(port->port.tty); } else { +out: gs_free_requests(ep, head, &port->read_allocated); gs_free_requests(port->port_usb->in, &port->write_pool, &port->write_allocated);