[PATCH v2 0/2] usb: gadget: u_serial: flush_buffer, and keep the port for its tty

[PATCH v2 0/2] usb: gadget: u_serial: flush_buffer, and keep the port for its tty

[Mehmet Fide]

From: Mehmet Fide <mehmet.fide@screeningeagle.com>

On a gadget whose host side does not read the port, the last close of
/dev/ttyGS* waits GS_CLOSE_TIMEOUT (15 s) for the write buffer to drain,
and tcflush(TCOFLUSH) cannot avoid that because u_serial has no
flush_buffer operation. v1 added it.

Sashiko pointed out that the hangup gserial_disconnect() schedules can
then call flush_buffer after gserial_free_line() has freed the port.
That window is older than the new callback: the tty core already reads
the freed port there, KASAN reports it in check_tty_count() with a
delay added to gs_close(). Patch 1 makes the port live as long as its
tty, with a tty_port reference taken in install and dropped in cleanup
as cdc-acm does, and carries Fixes and Cc stable. Patch 2 is v1.

Changes in v2:
- new patch 1, the port lifetime fix
- patch 2 unchanged

v1: https://lore.kernel.org/linux-usb/20261015162732.3170938-1-mehmet.fide@gmail.com/

Tested on a Verdin AM62 with KASAN and lockdep, 6.12.113 plus both
patches: with a 3 s delay in gs_close() the hangup after
gserial_free_line() no longer reports a use-after-free and the port is
freed after the tty; four function teardowns, two with the port open,
free four ports without leaks or warnings; close after tcflush returns
at once with the host not reading and the data written after the flush
arrives; 47850 flushes against a concurrent writer; NCM torn down
under iperf traffic. The same two patches run in our i.MX8MP and VF50
product images on CI rigs, gadget replug included. On usb-next both
patches build with W=1 and sparse without new warnings, each on its own.

Mehmet Fide (2):
  usb: gadget: u_serial: keep the port until its tty is released
  usb: gadget: u_serial: implement flush_buffer

 drivers/usb/gadget/function/u_serial.c | 59 +++++++++++++++++++++++++-
 1 file changed, 57 insertions(+), 2 deletions(-)


base-commit: 04ac7b445880b0878c83e6e89d961455cbea0427
-- 
2.55.0

[PATCH v2 1/2] usb: gadget: u_serial: keep the port until its tty is released

[Mehmet Fide]

From: Mehmet Fide <mehmet.fide@screeningeagle.com>

gserial_free_line() frees the gs_port, and with it the tty_port it
embeds, as soon as the last close has returned. The tty core keeps
using tty->port until release_tty(), so a hangup that
gserial_disconnect() scheduled before that close can still run and
read the freed port:

  BUG: KASAN: slab-use-after-free in check_tty_count+0x230/0x278
  Workqueue: events do_tty_hangup
   __tty_hangup.part.0
   do_tty_hangup
  Allocated by: gserial_alloc_line_no_console <- acm_alloc_instance
  Freed by: gserial_free_port <- gserial_free_line <- acm_free_instance

and release_tty() then cancels the buffer work of the freed port.

Take a reference on the tty_port in install and drop it in cleanup,
and let the last reference free the gs_port.

Fixes: 19b10a8828a6 ("usb: gadget: allocate & giveback serial ports instead hard code them")
Cc: stable@vger.kernel.org
Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
---

Notes:
    Changes in v2:
    - new patch: Sashiko pointed out that with flush_buffer the hangup that
      gserial_disconnect() schedules can run after gserial_free_line() has
      freed the port; the tty core itself already reads the freed port in that
      window (check_tty_count, release_tty), so the port has to live as long
      as its tty

 drivers/usb/gadget/function/u_serial.c | 45 ++++++++++++++++++++++++--
 1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
index cdd1dfc666c4..19eea724280b 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -908,7 +908,39 @@ 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		ret;
+
+	mutex_lock(&ports[port_num].lock);
+	port = ports[port_num].port;
+	if (port)
+		tty_port_get(&port->port);
+	mutex_unlock(&ports[port_num].lock);
+	if (!port)
+		return -ENODEV;
+
+	ret = tty_port_install(&port->port, driver, tty);
+	if (ret) {
+		tty_port_put(&port->port);
+		return ret;
+	}
+	tty->driver_data = port;
+	return 0;
+}
+
+static void gs_cleanup(struct tty_struct *tty)
+{
+	struct gs_port	*port = tty->driver_data;
+
+	tty_port_put(&port->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 +1235,15 @@ static void gs_console_exit(struct gs_port *port)
 
 #endif
 
+static void gs_port_destruct(struct tty_port *tport)
+{
+	kfree(container_of(tport, struct gs_port, port));
+}
+
+static const struct tty_port_operations gs_port_ops = {
+	.destruct = gs_port_destruct,
+};
+
 static int
 gs_port_alloc(unsigned port_num, struct usb_cdc_line_coding *coding)
 {
@@ -1222,6 +1263,7 @@ gs_port_alloc(unsigned port_num, struct usb_cdc_line_coding *coding)
 	}
 
 	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);
@@ -1258,8 +1300,7 @@ static void gserial_free_port(struct gs_port *port)
 	/* 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)
-- 
2.55.0

[PATCH v2 2/2] usb: gadget: u_serial: implement flush_buffer

[Mehmet Fide]

From: Mehmet Fide <mehmet.fide@screeningeagle.com>

gs_close() waits up to GS_CLOSE_TIMEOUT (15 s) for the write buffer
to drain while the gadget is connected. When the host does not read
the port, for example because no application on the host has it open,
the last close blocks for the full timeout. tcflush(TCOFLUSH) cannot
avoid that: u_serial has no flush_buffer operation, so the request is
a no-op.

Implement flush_buffer: discard the buffered output under the port
lock and wake up the close and write waiters. As on other tty drivers,
a hangup and an interrupt character (ISIG without NOFLSH) now discard
the pending output as well.

Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
---

Notes:
    Changes in v2:
    - none, now after the patch that keeps the port alive for the hangup

 drivers/usb/gadget/function/u_serial.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
index 19eea724280b..187a2598181a 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -822,6 +822,19 @@ static void gs_flush_chars(struct tty_struct *tty)
 	spin_unlock_irqrestore(&port->port_lock, flags);
 }
 
+static void gs_flush_buffer(struct tty_struct *tty)
+{
+	struct gs_port	*port = tty->driver_data;
+	unsigned long	flags;
+
+	spin_lock_irqsave(&port->port_lock, flags);
+	kfifo_reset(&port->port_write_buf);
+	spin_unlock_irqrestore(&port->port_lock, flags);
+
+	wake_up_interruptible(&port->drain_wait);
+	tty_wakeup(tty);
+}
+
 static unsigned int gs_write_room(struct tty_struct *tty)
 {
 	struct gs_port	*port = tty->driver_data;
@@ -946,6 +959,7 @@ static const struct tty_operations gs_tty_ops = {
 	.write =		gs_write,
 	.put_char =		gs_put_char,
 	.flush_chars =		gs_flush_chars,
+	.flush_buffer =		gs_flush_buffer,
 	.write_room =		gs_write_room,
 	.chars_in_buffer =	gs_chars_in_buffer,
 	.unthrottle =		gs_unthrottle,
-- 
2.55.0