Re: [PATCH] usb: gadget: u_serial: fix use-after-free between tty open/close and gserial_free_line

[Greg Kroah-Hartman]

On Thu, Sep 10, 2026 at 01:17:19PM +0500, Syed Tayyab Farooq wrote:
> syzbot reports a slab-use-after-free in tty_init_dev()/gs_close()
> involving struct gs_port. The port is allocated when a gadget serial
> function instance is created via configfs mkdir (gserial_alloc_line())
> and freed via a plain kfree() in gserial_free_port() when the instance
> is removed via configfs rmdir(). Nothing prevented a concurrent
> open("/dev/ttyGS*") from racing with rmdir: the tty core could still
> reach gs_open()/gs_close() and dereference the gs_port after it had
> already been freed, since the ports[] table entry these functions
> looked up was a bare pointer with no refcounting tied to the tty core.
> 
> Fix this by giving struct gs_port proper tty_port-managed lifetime:
> 
>  - Add gs_install()/gs_cleanup() tty_operations. gs_install() looks up
>    the port once under ports[idx].lock, takes a tty_port_get()
>    reference, and stores the result in tty->driver_data. gs_cleanup()
>    drops that reference when the tty_struct is released. This ties the
>    gs_port's minimum lifetime to the tty_struct using it, so it can no
>    longer be freed out from under an open tty.
> 
>  - Give tty_port a destructor (gs_port_destruct()) that does the
>    kfree() that gserial_free_port() used to do directly, and
>    switch gserial_free_port() to tty_port_put() instead of an
>    unconditional kfree(). Also, tty_port_destroy is automatically called
>    when the reference reaches 0. The struct is now only actually freed once
>    its last reference (held by either the ports[] table or a live tty)
>    is dropped.
> 
>  - Stop gs_open() from independently re-deriving the port from
>    ports[port_num].port and reassigning tty->driver_data. gs_install()
>    is now the single place that looks up and pins the port; gs_open()
>    re-deriving it separately could, on an unlucky race with the port
>    index being freed and reallocated, leave tty->port and
>    tty->driver_data pointing at two different gs_port instances, with
>    the one gs_close() uses left unprotected. gs_open() now just uses
>    the already-validated tty->driver_data, while still holding
>    ports[port_num].lock across the first-open kfifo allocation to
>    serialize concurrent first opens against each other (kfifo_alloc()
>    needs GFP_KERNEL, so it can't run under port_lock).
> 
>  - In gs_close(), a tty's .close() can legitimately run against a port
>    whose port.count is still 0, e.g. when gs_open() fails and the tty
>    core unwinds via tty_release(). The previous code treated this as
>    an impossible state (WARN_ON(1)), which trips panic_on_warn on
>    syzbot and isn't actually a bug -- it's an expected outcome of a
>    failed open. Treat count == 0 the same as any other "not the last
>    closer" case and return quietly instead of warning.
> 
> Reported-by: syzbot+fe63e4d633540f230624@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=fe63e4d633540f230624
> Fixes: c1dca562be8a ("usb gadget: split out serial core")
> Signed-off-by: Syed Tayyab Farooq <syedtayyabfarooq08@xxxxxxxxx>
> ---
>  drivers/usb/gadget/function/u_serial.c | 64 +++++++++++++++++++++++---
>  1 file changed, 57 insertions(+), 7 deletions(-)

Did you forget an Assisted-by: tag?  And did syzbot test this?

thanks,

greg k-h