* [PATCH v3 0/2] usb: gadget: u_serial: fix gs_port use-after-free on open/unbind race @ 2026-11-09 14:11 Varun Singh 2026-11-09 14:11 ` [PATCH v3 1/2] usb: gadget: u_serial: serialise line setup against line teardown Varun Singh 2026-11-09 14:11 ` [PATCH v3 2/2] usb: gadget: u_serial: fix gs_port use-after-free on open/unbind race Varun Singh 0 siblings, 2 replies; 5+ messages in thread From: Varun Singh @ 2026-11-09 14:11 UTC (permalink / raw) To: nkapron, gregkh Cc: linux-usb, linux-kernel, bigeasy, syzbot+39a2135de1558208d3b3, 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 1/2] usb: gadget: u_serial: serialise line setup against line teardown 2026-11-09 14:11 [PATCH v3 0/2] usb: gadget: u_serial: fix gs_port use-after-free on open/unbind race Varun Singh @ 2026-11-09 14:11 ` Varun Singh 2026-11-09 14:28 ` sashiko-bot 2026-11-09 14:11 ` [PATCH v3 2/2] usb: gadget: u_serial: fix gs_port use-after-free on open/unbind race Varun Singh 1 sibling, 1 reply; 5+ messages in thread From: Varun Singh @ 2026-11-09 14:11 UTC (permalink / raw) To: nkapron, gregkh Cc: linux-usb, linux-kernel, bigeasy, syzbot+39a2135de1558208d3b3, 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/2] usb: gadget: u_serial: serialise line setup against line teardown 2026-11-09 14:11 ` [PATCH v3 1/2] usb: gadget: u_serial: serialise line setup against line teardown Varun Singh @ 2026-11-09 14:28 ` sashiko-bot 0 siblings, 0 replies; 5+ messages in thread From: sashiko-bot @ 2026-11-09 14:28 UTC (permalink / raw) To: Varun Singh; +Cc: linux-usb Thank you for your contribution! Sashiko AI review found 1 potential issue to consider: - [Low] usb: gadget: u_serial: missing Fixes trailer for commit 19b10a8828a6 -- commit 7fc863482be78ba089c79dd3a13d5ea59b19a79c Author: Varun Singh <varun.singh10011@gmail.com> > usb: gadget: u_serial: serialise line setup against line teardown > > 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> [Severity: Low] Should this bug fix include a Fixes: tag for the commit that introduced dynamic serial line allocation and teardown without holding ports[port_num].lock across tty device registration and unregistration? Fixes: 19b10a8828a6 ("usb: gadget: allocate & giveback serial ports instead hard code them") -- Sashiko AI review · https://sashiko.dev/#/patchset/20261109141110.3538-1-varun.singh10011@gmail.com?part=1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 2/2] usb: gadget: u_serial: fix gs_port use-after-free on open/unbind race 2026-11-09 14:11 [PATCH v3 0/2] usb: gadget: u_serial: fix gs_port use-after-free on open/unbind race Varun Singh 2026-11-09 14:11 ` [PATCH v3 1/2] usb: gadget: u_serial: serialise line setup against line teardown Varun Singh @ 2026-11-09 14:11 ` Varun Singh 2026-11-09 14:33 ` sashiko-bot 1 sibling, 1 reply; 5+ messages in thread From: Varun Singh @ 2026-11-09 14:11 UTC (permalink / raw) To: nkapron, gregkh Cc: linux-usb, linux-kernel, bigeasy, syzbot+39a2135de1558208d3b3, 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 2/2] usb: gadget: u_serial: fix gs_port use-after-free on open/unbind race 2026-11-09 14:11 ` [PATCH v3 2/2] usb: gadget: u_serial: fix gs_port use-after-free on open/unbind race Varun Singh @ 2026-11-09 14:33 ` sashiko-bot 0 siblings, 0 replies; 5+ messages in thread From: sashiko-bot @ 2026-11-09 14:33 UTC (permalink / raw) To: Varun Singh; +Cc: linux-usb Thank you for your contribution! Sashiko AI review found 3 potential issues to consider: - [High] usb: gadget: u_serial: tty_struct UAF in gserial_free_line() via vhangup - [High] usb: gadget: u_serial: premature teardown in gs_close() on failed reopen - [Medium] usb: gadget: u_serial: stale tty in ttys[] blocks open after line reuse -- commit ce18206b8942dfa93fb6a098764986e87bedbf4e Author: Varun Singh <varun.singh10011@gmail.com> usb: gadget: u_serial: fix gs_port use-after-free on open/unbind race This commit switches gs_port lifetime management to the embedded tty_port kref via tty_operations.install and .cleanup callbacks, removes the close_wait synchronization in gserial_free_port(), and hangs up the port in gserial_free_line() when tearing down a serial line. > diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c > index e172050f13927..f279a2c6507c6 100644 > --- a/drivers/usb/gadget/function/u_serial.c > +++ b/drivers/usb/gadget/function/u_serial.c [ ... ] > @@ -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; > } [Severity: Medium] If a line is freed and reallocated while a process still holds an open file descriptor from the previous instance, does this check make the newly registered /dev/ttyGS* device unopenable until the old descriptor is closed? tty_port_tty_vhangup() marks the old tty_struct as hung up and kills its line discipline, but it does not remove the tty_struct from gs_tty_driver->ttys[port_num]; only release_tty() clears that slot when the last open file descriptor is closed. When gserial_alloc_line() reuses port_num in the meantime, any new open() on /dev/ttyGS* finds the old tty_struct in gs_tty_driver->ttys[port_num] via tty_driver_lookup_tty() and calls tty_reopen() instead of gs_install(). Because tty->driver_data still points to the old gs_port while ports[port_num].port points to the new one, every open() on the new device fails with -ENODEV, and tty_reopen() also reinitializes an active line discipline on the hung-up tty_struct via tty_ldisc_reinit() that remains attached until the old file descriptor is closed. [ ... ] > @@ -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; > } [Severity: High] Can a failed reopen in gs_open() decrement port->port.count or run final close teardown in gs_close() while another process still has the port open? When gs_open() returns -ENODEV before incrementing port->port.count, tty_open() calls tty_release() -> gs_close(). If another process already holds an open file descriptor on the same tty, port->port.count is at least 1 when the second opener takes tty_reopen() and fails in gs_open(): CPU0 (process A) CPU1 (process B) tty_open() gs_install() gs_open() port->port.count++ (now 1) gserial_free_line() ports[port_num].port = NULL tty_open() tty_reopen() gs_open() returns -ENODEV tty_release() gs_close() /* port->port.count == 1 */ kfifo_free(&port->port_write_buf) port->port.count = 0 port->port.tty = NULL gserial_free_line() continues: tty_port_tty_vhangup() /* port->port.tty is NULL, skips */ gs_write() / gs_put_char() kfifo_put(&port->port_write_buf, ...) <-- NULL pointer dereference If port->port.count was greater than 1, the failed reopen decrements an active opener's count; if port->port.count was 1, gs_close() frees port_write_buf and clears port->port.tty while process A's descriptor is still open, preventing tty_port_tty_vhangup() from hanging up process A's tty and causing subsequent writes to dereference a NULL kfifo buffer. [ ... ] > @@ -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); [Severity: High] Can tty_port_tty_vhangup() race with gs_close() and cause a use-after-free and double free of struct tty_struct? tty_port_tty_vhangup() calls tty_port_tty_get(), which locks port->port.lock (struct tty_port::lock) and calls tty_kref_get(port->tty), assuming port->tty holds a reference via tty_port_tty_set(). However, u_serial never calls tty_port_tty_set(): gs_open() assigns port->port.tty = tty without taking a kref, and gs_close() clears port->port.tty = NULL under port->port_lock (struct gs_port::port_lock) rather than port->port.lock. When gserial_free_line() races with the final close of /dev/ttyGS*: CPU0 CPU1 tty_release() gserial_free_line() gs_close() tty_port_tty_vhangup(&port->port) spin_lock_irq(&port->port_lock) tty_port_tty_get(&port->port) spin_lock_irqsave(&port->port.lock) /* reads non-NULL port->port.tty */ port->port.tty = NULL spin_unlock_irq(&port->port_lock) tty_release_struct() release_tty() tty_kref_put(tty) -> 0 tty_kref_get(tty) <-- refcount 0->1 tty_vhangup(tty) <-- use-after-free tty_kref_put(tty) <-- double free Using tty_port_tty_set() in gs_open() and gs_close() would maintain the tty kref and serialize port->port.tty updates under port->port.lock. -- Sashiko AI review · https://sashiko.dev/#/patchset/20261109141110.3538-1-varun.singh10011@gmail.com?part=2 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-11-09 14:33 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-11-09 14:11 [PATCH v3 0/2] usb: gadget: u_serial: fix gs_port use-after-free on open/unbind race Varun Singh 2026-11-09 14:11 ` [PATCH v3 1/2] usb: gadget: u_serial: serialise line setup against line teardown Varun Singh 2026-11-09 14:28 ` sashiko-bot 2026-11-09 14:11 ` [PATCH v3 2/2] usb: gadget: u_serial: fix gs_port use-after-free on open/unbind race Varun Singh 2026-11-09 14:33 ` sashiko-bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox