FROMLIST: usb: gadget: u_serial: Add null pointer checks after RX/TX submission

Commit ffd603f214 ("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: ffd603f214 ("usb: gadget: u_serial: Add null pointer check in gs_start_io")
Cc: stable@vger.kernel.org
Signed-off-by: Kuen-Han Tsai <khtsai@google.com>

Bug: 283247551
Link: https://lore.kernel.org/lkml/20240116141801.396398-1-khtsai@google.com/
Change-Id: Ib850c7d313194074941576a7fdd3a9f58486ad78
Signed-off-by: Kuen-Han Tsai <khtsai@google.com>
This commit is contained in:
Kuen-Han Tsai
2024-01-16 22:16:17 +08:00
committed by Matthias Männich
parent 7de5ae52b1
commit c78828e383

View File

@@ -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);