[PATCH v3 0/2] usb: gadget: u_serial: fix gs_port use-after-free on open/unbind race

[PATCH v3 0/2] usb: gadget: u_serial: fix gs_port use-after-free on open/unbind race

[Varun Singh]

2/2 fixes the use-after-free reported by syzbot in check_tty_count(). It
is based on v2, with the device unregistration change moved into 1/2.

1/2 is new. While reviewing v2 I noticed that ports[n].lock protected the
ports[n].port slot but not the matching tty device registration, letting
concurrent function creation and removal race so that creation fails with
-EEXIST. That is independent of the reported use-after-free, but 2/2
relies on the slot and the device changing together, so it goes first.

v2 -> v3:
 - new 1/2: hold ports[n].lock across slot assignment and device
   registration, and across slot clearing and device unregistration
 - 2/2: unregister-before-free moved into 1/2; otherwise unchanged

v1 -> v2:
 - look the port up in .install and hold a tty_port reference for the
   lifetime of the tty, instead of clearing tty->driver_data on close
 - drop the close_wait handshake: with a correct port count it blocks
   rmdir until userspace closes /dev/ttyGS*, seen as a hung task
 - hang up any tty still installed on a freed line, and recheck in
   gs_open() that tty->driver_data matches the registered port

Testing, in QEMU against a configfs mkdir/rmdir and open() race
reproducer, A/B against unmodified master each time:
 - clean under KASAN and, separately, under lockdep (PROVE_LOCKING,
   DEBUG_ATOMIC_SLEEP, DETECT_HUNG_TASK); kmemleak reported no leaks
 - slab fault injection exercised 1/2's registration-failure rollback
   107 times, after which the line was still allocatable and openable
 - 1/2 was built and booted on its own, to check bisectability
 - the series passed #syz test on 2026-11-08 against syzbot's own
   reproducer, which covers the acm path the local one does not:
   "testing succeeded, no crash reproduced"

Not addressed here: gserial_alloc_line() re-reads ports[0].port after the
line's lock has been dropped and passes it to gs_console_init(), which has
no NULL check, so a concurrent gserial_free_line(0) can leave it NULL.
This predates the series. I have left it out rather than pull
register_console() under ports[n].lock without investigating that further,
and can send a separate patch for it.

Link: https://lore.kernel.org/all/20261102193436.6044-1-varun.singh10011@gmail.com/ [v2]

Varun Singh (2):
  usb: gadget: u_serial: serialise line setup against line teardown
  usb: gadget: u_serial: fix gs_port use-after-free on open/unbind race

 drivers/usb/gadget/function/u_serial.c | 132 ++++++++++++++++---------
 1 file changed, 85 insertions(+), 47 deletions(-)

-- 
2.34.1

[PATCH v3 1/2] usb: gadget: u_serial: serialise line setup against line teardown

[Varun Singh]

Creating a u_serial gadget function can fail with -EEXIST for no reason:

  sysfs: cannot create duplicate filename '/devices/virtual/tty/ttyGS0'
  gserial_alloc_line_no_console: failed to register tty for port 0, err -17

ports[n].lock protects the ports[n].port slot, but the line's tty device
is registered and unregistered outside it, so the two can change
independently:

  gserial_alloc_line_no_console()        gserial_free_line()
    gs_port_alloc()
      mutex_lock(ports[n].lock)
                                           mutex_lock(ports[n].lock)
                                           ports[n].port = NULL
                                           mutex_unlock(ports[n].lock)
      ports[n].port = new
      mutex_unlock(ports[n].lock)
    tty_port_register_device(n)            <- device for n still there
                                           tty_unregister_device(n)

The allocator claims a slot whose old device is still registered, which
is the -EEXIST above. The reverse order leaves a slot with no device
node.

Hold ports[n].lock across slot assignment and device registration, and
across slot clearing and unregistration, so a concurrent caller sees
either both or neither. gs_port_alloc() is now called with the lock
held. Neither tty_port_register_device() nor tty_unregister_device()
takes tty_lock or ports[n].lock, and gserial_free_line() already slept
under this mutex, so this adds no new lock dependency.

With two gadgets racing for line 0, 25 of 383 function creations failed
on an unpatched kernel; 0 of 530 with this patch.

Found with the assistance of Claude Opus 5.

Signed-off-by: Varun Singh <varun.singh10011@gmail.com>
---
 drivers/usb/gadget/function/u_serial.c | 54 ++++++++++++++------------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
index cdd1dfc666c4..e172050f1392 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1203,23 +1203,18 @@ static void gs_console_exit(struct gs_port *port)
 
 #endif
 
+/* Caller must hold ports[port_num].lock. */
 static int
 gs_port_alloc(unsigned port_num, struct usb_cdc_line_coding *coding)
 {
 	struct gs_port	*port;
-	int		ret = 0;
 
-	mutex_lock(&ports[port_num].lock);
-	if (ports[port_num].port) {
-		ret = -EBUSY;
-		goto out;
-	}
+	if (ports[port_num].port)
+		return -EBUSY;
 
 	port = kzalloc_obj(struct gs_port);
-	if (port == NULL) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	if (!port)
+		return -ENOMEM;
 
 	tty_port_init(&port->port);
 	spin_lock_init(&port->port_lock);
@@ -1236,9 +1231,8 @@ gs_port_alloc(unsigned port_num, struct usb_cdc_line_coding *coding)
 	port->port_line_coding = *coding;
 
 	ports[port_num].port = port;
-out:
-	mutex_unlock(&ports[port_num].lock);
-	return ret;
+
+	return 0;
 }
 
 static int gs_closed(struct gs_port *port)
@@ -1267,17 +1261,23 @@ void gserial_free_line(unsigned char port_num)
 	struct gs_port	*port;
 
 	mutex_lock(&ports[port_num].lock);
-	if (!ports[port_num].port) {
+	port = ports[port_num].port;
+	if (!port) {
 		mutex_unlock(&ports[port_num].lock);
 		return;
 	}
-	port = ports[port_num].port;
 	gs_console_exit(port);
 	ports[port_num].port = NULL;
+
+	/*
+	 * Unregister under the lock, so the device cannot outlive its slot:
+	 * otherwise a concurrent gserial_alloc_line() could claim the line
+	 * and register a device that this unregister would tear down.
+	 */
+	tty_unregister_device(gs_tty_driver, port_num);
 	mutex_unlock(&ports[port_num].lock);
 
 	gserial_free_port(port);
-	tty_unregister_device(gs_tty_driver, port_num);
 }
 EXPORT_SYMBOL_GPL(gserial_free_line);
 
@@ -1294,13 +1294,18 @@ int gserial_alloc_line_no_console(unsigned char *line_num)
 	coding.bParityType = USB_CDC_NO_PARITY;
 	coding.bDataBits = USB_CDC_1_STOP_BITS;
 
+	/*
+	 * Hold the line's lock across both, so a concurrent
+	 * gserial_free_line() sees the slot and the device together.
+	 */
 	for (port_num = 0; port_num < MAX_U_SERIAL_PORTS; port_num++) {
+		mutex_lock(&ports[port_num].lock);
 		ret = gs_port_alloc(port_num, &coding);
-		if (ret == -EBUSY)
-			continue;
-		if (ret)
+		if (!ret)
+			break;
+		mutex_unlock(&ports[port_num].lock);
+		if (ret != -EBUSY)
 			return ret;
-		break;
 	}
 	if (ret)
 		return ret;
@@ -1315,15 +1320,16 @@ int gserial_alloc_line_no_console(unsigned char *line_num)
 				__func__, port_num, PTR_ERR(tty_dev));
 
 		ret = PTR_ERR(tty_dev);
-		mutex_lock(&ports[port_num].lock);
 		ports[port_num].port = NULL;
 		mutex_unlock(&ports[port_num].lock);
 		gserial_free_port(port);
-		goto err;
+		return ret;
 	}
+	mutex_unlock(&ports[port_num].lock);
+
 	*line_num = port_num;
-err:
-	return ret;
+
+	return 0;
 }
 EXPORT_SYMBOL_GPL(gserial_alloc_line_no_console);
 
-- 
2.34.1

[PATCH v3 2/2] usb: gadget: u_serial: fix gs_port use-after-free on open/unbind race

[Varun Singh]

syzbot reports a use-after-free when a gadget serial function is removed
through configfs while /dev/ttyGS* is being opened:

  BUG: KASAN: slab-use-after-free in check_tty_count+0x134/0x2b0
   tty_release+0x95/0x1300 drivers/tty/tty_io.c:1728

gs_port embeds a tty_port, which the TTY core stores in tty->port without
taking a reference, so the driver must keep it alive. u_serial instead
waited for port.count to reach zero in gserial_free_port() and then freed
the port. That never worked: once gserial_free_line() has cleared the
ports[] slot, gs_open() returns -ENODEV without incrementing the count, so
the wait completes immediately, the port is freed, and tty_release() reads
it.

Use the tty_port kref. gs_install() runs before the core publishes
tty->port, so it looks the port up under ports[].lock, takes a reference
and stores it in tty->driver_data; .cleanup drops it, .destruct frees the
port, and gserial_free_port() drops the allocation reference rather than
calling kfree().

The close_wait/gs_closed() handshake is then redundant, and once the count
is correct it blocks rmdir until userspace closes /dev/ttyGS*, which syzbot
saw as a hung task, so drop it. Keep cancel_delayed_work_sync(): cancelling
from .destruct instead leaves a use-after-free in the work cancellation
path.

Without the handshake a line can be freed and reallocated while an old
tty_struct is still in gs_tty_driver->ttys[], which only release_tty()
clears. A new open would find it via tty_driver_lookup_tty(), take
tty_reopen() instead of gs_install(), and run gs_open() against the
orphaned port. So gserial_free_line() now hangs up any tty still on the
line, as usb_serial_disconnect() does, and gs_open() rechecks that
tty->driver_data is still the port registered for the line.

gs_close() also no longer warns on a zero port.count: tty->driver_data is
now set before gs_open() runs, so tty_release() legitimately calls
->close() with a zero count after a failed open.

Reproduce by racing repeated open() of /dev/ttyGS0 against rmdir of a
u_serial gadget function, with some opens held so they overlap. An
unpatched KASAN kernel reports the use-after-free within a few thousand
rounds.

Found with the assistance of Claude Opus 5.

Fixes: 19b10a8828a6 ("usb: gadget: allocate & giveback serial ports instead hard code them")
Reported-by: syzbot+39a2135de1558208d3b3@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=39a2135de1558208d3b3
Tested-by: syzbot+39a2135de1558208d3b3@syzkaller.appspotmail.com
Signed-off-by: Varun Singh <varun.singh10011@gmail.com>
---
 drivers/usb/gadget/function/u_serial.c | 80 ++++++++++++++++++--------
 1 file changed, 56 insertions(+), 24 deletions(-)

diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
index e172050f1392..f279a2c6507c 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -125,7 +125,6 @@ struct gs_port {
 	struct kfifo		port_write_buf;
 	wait_queue_head_t	drain_wait;	/* wait while writes drain */
 	bool                    write_busy;
-	wait_queue_head_t	close_wait;
 	bool			suspended;	/* port suspended */
 	bool			start_delayed;	/* delay start when suspended */
 	struct async_icount	icount;
@@ -615,8 +614,9 @@ static int gs_open(struct tty_struct *tty, struct file *file)
 	int		status = 0;
 
 	mutex_lock(&ports[port_num].lock);
+	/* Reject opens on a removed port, or a stale tty after line reuse. */
 	port = ports[port_num].port;
-	if (!port) {
+	if (!port || port != tty->driver_data) {
 		status = -ENODEV;
 		goto out;
 	}
@@ -648,7 +648,6 @@ static int gs_open(struct tty_struct *tty, struct file *file)
 	if (port->port.count++)
 		goto exit_unlock_port;
 
-	tty->driver_data = port;
 	port->port.tty = tty;
 
 	/* if connected, start the I/O stream */
@@ -693,15 +692,14 @@ static int gs_close_flush_done(struct gs_port *p)
 static void gs_close(struct tty_struct *tty, struct file *file)
 {
 	struct gs_port *port = tty->driver_data;
-	struct gserial	*gser;
+	struct gserial *gser;
 
 	spin_lock_irq(&port->port_lock);
 
 	if (port->port.count != 1) {
 raced_with_open:
-		if (port->port.count == 0)
-			WARN_ON(1);
-		else
+		/* A failed open can still reach ->close() with a zero count. */
+		if (port->port.count)
 			--port->port.count;
 		goto exit;
 	}
@@ -744,7 +742,6 @@ static void gs_close(struct tty_struct *tty, struct file *file)
 	pr_debug("gs_close: ttyGS%d (%p,%p) done!\n",
 			port->port_num, tty, file);
 
-	wake_up(&port->close_wait);
 exit:
 	spin_unlock_irq(&port->port_lock);
 }
@@ -908,7 +905,40 @@ static int gs_get_icount(struct tty_struct *tty,
 	return 0;
 }
 
+static int gs_install(struct tty_driver *driver, struct tty_struct *tty)
+{
+	int port_num = tty->index;
+	struct gs_port *port;
+	int status;
+
+	mutex_lock(&ports[port_num].lock);
+	port = ports[port_num].port;
+	if (!port) {
+		mutex_unlock(&ports[port_num].lock);
+		return -ENODEV;
+	}
+	tty_port_get(&port->port);
+	mutex_unlock(&ports[port_num].lock);
+
+	status = tty_port_install(&port->port, driver, tty);
+	if (status) {
+		tty_port_put(&port->port);
+		return status;
+	}
+
+	tty->driver_data = port;
+
+	return 0;
+}
+
+static void gs_cleanup(struct tty_struct *tty)
+{
+	tty_port_put(tty->port);
+}
+
 static const struct tty_operations gs_tty_ops = {
+	.install =		gs_install,
+	.cleanup =		gs_cleanup,
 	.open =			gs_open,
 	.close =		gs_close,
 	.write =		gs_write,
@@ -1203,6 +1233,15 @@ static void gs_console_exit(struct gs_port *port)
 
 #endif
 
+static void gs_destruct_port(struct tty_port *port)
+{
+	kfree(container_of(port, struct gs_port, port));
+}
+
+static const struct tty_port_operations gs_port_ops = {
+	.destruct = gs_destruct_port,
+};
+
 /* Caller must hold ports[port_num].lock. */
 static int
 gs_port_alloc(unsigned port_num, struct usb_cdc_line_coding *coding)
@@ -1217,9 +1256,9 @@ gs_port_alloc(unsigned port_num, struct usb_cdc_line_coding *coding)
 		return -ENOMEM;
 
 	tty_port_init(&port->port);
+	port->port.ops = &gs_port_ops;
 	spin_lock_init(&port->port_lock);
 	init_waitqueue_head(&port->drain_wait);
-	init_waitqueue_head(&port->close_wait);
 
 	INIT_DELAYED_WORK(&port->push, gs_rx_push);
 
@@ -1235,25 +1274,11 @@ gs_port_alloc(unsigned port_num, struct usb_cdc_line_coding *coding)
 	return 0;
 }
 
-static int gs_closed(struct gs_port *port)
-{
-	int cond;
-
-	spin_lock_irq(&port->port_lock);
-	cond = port->port.count == 0;
-	spin_unlock_irq(&port->port_lock);
-
-	return cond;
-}
-
 static void gserial_free_port(struct gs_port *port)
 {
 	cancel_delayed_work_sync(&port->push);
-	/* wait for old opens to finish */
-	wait_event(port->close_wait, gs_closed(port));
 	WARN_ON(port->port_usb != NULL);
-	tty_port_destroy(&port->port);
-	kfree(port);
+	tty_port_put(&port->port);
 }
 
 void gserial_free_line(unsigned char port_num)
@@ -1277,6 +1302,13 @@ void gserial_free_line(unsigned char port_num)
 	tty_unregister_device(gs_tty_driver, port_num);
 	mutex_unlock(&ports[port_num].lock);
 
+	/*
+	 * Hang up the tty before the line can be reused. Drop
+	 * ports[port_num].lock first: it takes tty_lock, which gs_install()
+	 * already holds when it acquires that mutex.
+	 */
+	tty_port_tty_vhangup(&port->port);
+
 	gserial_free_port(port);
 }
 EXPORT_SYMBOL_GPL(gserial_free_line);
-- 
2.34.1