* [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
* [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 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
* 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