Message-ID: <ea1a13ad-a1e0-540a-e97a-4c44f6d2d33b@0882a8b5-c6c3-11e9-b005-00805fc181fe.uuid.home.arpa>
Date:   Wed, 23 Aug 2023 21:37:45 +0100
From:   Simon Arlott <simon@...iron.net>
To:     Oliver Neukum <oneukum@...e.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jirislaby@...nel.org>
Cc:     linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
        linux-serial@...r.kernel.org
Subject: [PATCH] USB: cdc-acm: expose serial close_delay and closing_wait in
 sysfs

If the serial device never reads data written to it (because it is "output
only") then the write buffers will still be waiting for the URB to complete
on close(), which will hang for 30s until the closing_wait timeout expires.

This can happen with the ESP32-H2/ESP32-C6 USB serial interface. Instead of
changing all userspace applications to flush (discard) their output in this
specific scenario it would be easier to adjust the closing_wait timeout
with udev rules but the only available interface is the TIOCGSERIAL ioctl.

The serial_core driver (ttySx) exposes its supported ioctl values as
read-only sysfs attributes. Add read-write sysfs attributes "close_delay"
and "closing_wait" to cdc-acm (ttyACMx) devices. These are the same as the
attributes in serial_core except that the "closing_wait" sysfs values are
modified so that "-1" is used for "infinite wait" (instead of 0) and "0"
is used for "no wait" (instead of 65535).

Signed-off-by: Simon Arlott <simon@...iron.net>
---
 Documentation/ABI/testing/sysfs-tty |  21 +++++
 drivers/usb/class/cdc-acm.c         | 135 +++++++++++++++++++++++++---
 2 files changed, 144 insertions(+), 12 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-tty b/Documentation/ABI/testing/sysfs-tty
index 820e412d38a8..e04e322af568 100644
--- a/Documentation/ABI/testing/sysfs-tty
+++ b/Documentation/ABI/testing/sysfs-tty
@@ -161,3 +161,24 @@ Contact:	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
 Description:
 		 Allows user to detach or attach back the given device as
 		 kernel console. It shows and accepts a boolean variable.
+
+What:		/sys/class/tty/ttyACM0/close_delay
+Date:		August 2023
+Contact:	linux-usb@...r.kernel.org
+Description:
+		Set the closing delay time for this port in ms.
+
+		These sysfs values expose the TIOCGSERIAL interface via
+		sysfs rather than via ioctls.
+
+What:		/sys/class/tty/ttyACM0/closing_wait
+Date:		August 2023
+Contact:	linux-usb@...r.kernel.org
+Description:
+		Set the close wait time for this port in ms.
+
+		These sysfs values expose the TIOCGSERIAL interface via
+		sysfs rather than via ioctls.
+
+		Unlike the ioctl interface, waiting forever is represented as
+		-1 and zero is used to disable waiting on close.
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 00db9e1fc7ed..07e805df5113 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -953,21 +953,18 @@ static int acm_tty_tiocmset(struct tty_struct *tty,
 	return acm_set_control(acm, acm->ctrlout = newctrl);
 }
 
-static int get_serial_info(struct tty_struct *tty, struct serial_struct *ss)
+static void acm_read_serial_info(struct acm *acm, struct serial_struct *ss)
 {
-	struct acm *acm = tty->driver_data;
-
 	ss->line = acm->minor;
 	ss->close_delay	= jiffies_to_msecs(acm->port.close_delay) / 10;
 	ss->closing_wait = acm->port.closing_wait == ASYNC_CLOSING_WAIT_NONE ?
 				ASYNC_CLOSING_WAIT_NONE :
 				jiffies_to_msecs(acm->port.closing_wait) / 10;
-	return 0;
 }
 
-static int set_serial_info(struct tty_struct *tty, struct serial_struct *ss)
+static int acm_write_serial_info(struct acm *acm, struct serial_struct *ss,
+	bool admin)
 {
-	struct acm *acm = tty->driver_data;
 	unsigned int closing_wait, close_delay;
 	int retval = 0;
 
@@ -976,9 +973,7 @@ static int set_serial_info(struct tty_struct *tty, struct serial_struct *ss)
 			ASYNC_CLOSING_WAIT_NONE :
 			msecs_to_jiffies(ss->closing_wait * 10);
 
-	mutex_lock(&acm->port.mutex);
-
-	if (!capable(CAP_SYS_ADMIN)) {
+	if (!admin) {
 		if ((close_delay != acm->port.close_delay) ||
 		    (closing_wait != acm->port.closing_wait))
 			retval = -EPERM;
@@ -987,8 +982,28 @@ static int set_serial_info(struct tty_struct *tty, struct serial_struct *ss)
 		acm->port.closing_wait = closing_wait;
 	}
 
+	return 0;
+}
+
+static int get_serial_info(struct tty_struct *tty, struct serial_struct *ss)
+{
+	struct acm *acm = tty->driver_data;
+
+	mutex_lock(&acm->port.mutex);
+	acm_read_serial_info(acm, ss);
 	mutex_unlock(&acm->port.mutex);
-	return retval;
+	return 0;
+}
+
+static int set_serial_info(struct tty_struct *tty, struct serial_struct *ss)
+{
+	struct acm *acm = tty->driver_data;
+	int ret = 0;
+
+	mutex_lock(&acm->port.mutex);
+	ret = acm_write_serial_info(acm, ss, capable(CAP_SYS_ADMIN));
+	mutex_unlock(&acm->port.mutex);
+	return ret;
 }
 
 static int wait_serial_change(struct acm *acm, unsigned long arg)
@@ -1162,6 +1177,102 @@ static int acm_write_buffers_alloc(struct acm *acm)
 	return 0;
 }
 
+static ssize_t close_delay_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct acm *acm = dev_get_drvdata(dev);
+	struct serial_struct ss;
+
+	mutex_lock(&acm->port.mutex);
+	acm_read_serial_info(acm, &ss);
+	mutex_unlock(&acm->port.mutex);
+
+	return snprintf(buf, PAGE_SIZE, "%u\n", ss.close_delay);
+}
+
+static ssize_t close_delay_store(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct acm *acm = dev_get_drvdata(dev);
+	struct serial_struct ss;
+	u16 close_delay;
+	int ret;
+
+	ret = kstrtou16(buf, 0, &close_delay);
+	if (ret)
+		return ret;
+
+	mutex_lock(&acm->port.mutex);
+	acm_read_serial_info(acm, &ss);
+	ss.close_delay = close_delay;
+	ret = acm_write_serial_info(acm, &ss, true);
+	mutex_unlock(&acm->port.mutex);
+
+	return ret ? ret : count;
+}
+
+static ssize_t closing_wait_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct acm *acm = dev_get_drvdata(dev);
+	struct serial_struct ss;
+	s32 value;
+
+	mutex_lock(&acm->port.mutex);
+	acm_read_serial_info(acm, &ss);
+	mutex_unlock(&acm->port.mutex);
+
+	if (ss.closing_wait == ASYNC_CLOSING_WAIT_NONE)
+		value = 0;
+	else if (ss.closing_wait == ASYNC_CLOSING_WAIT_INF)
+		value = -1;
+	else
+		value = ss.closing_wait;
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", value);
+}
+
+static ssize_t closing_wait_store(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct acm *acm = dev_get_drvdata(dev);
+	struct serial_struct ss;
+	s32 closing_wait;
+	int ret;
+
+	ret = kstrtos32(buf, 0, &closing_wait);
+	if (ret)
+		return ret;
+
+	if (closing_wait == 0) {
+		closing_wait = ASYNC_CLOSING_WAIT_NONE;
+	} else if (closing_wait == -1) {
+		closing_wait = ASYNC_CLOSING_WAIT_INF;
+	} else if (closing_wait == ASYNC_CLOSING_WAIT_NONE
+			|| closing_wait == ASYNC_CLOSING_WAIT_INF /* redundant (0) */
+			|| closing_wait < -1 || closing_wait > U16_MAX) {
+		return -ERANGE;
+	}
+
+	mutex_lock(&acm->port.mutex);
+	acm_read_serial_info(acm, &ss);
+	ss.closing_wait = closing_wait;
+	ret = acm_write_serial_info(acm, &ss, true);
+	mutex_unlock(&acm->port.mutex);
+
+	return ret ? ret : count;
+}
+
+static DEVICE_ATTR_RW(close_delay);
+static DEVICE_ATTR_RW(closing_wait);
+
+static struct attribute *tty_dev_attrs[] = {
+	&dev_attr_close_delay.attr,
+	&dev_attr_closing_wait.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(tty_dev);
+
 static int acm_probe(struct usb_interface *intf,
 		     const struct usb_device_id *id)
 {
@@ -1503,8 +1614,8 @@ static int acm_probe(struct usb_interface *intf,
 			goto err_remove_files;
 	}
 
-	tty_dev = tty_port_register_device(&acm->port, acm_tty_driver, minor,
-			&control_interface->dev);
+	tty_dev = tty_port_register_device_attr(&acm->port, acm_tty_driver,
+			minor, &control_interface->dev, acm, tty_dev_groups);
 	if (IS_ERR(tty_dev)) {
 		rv = PTR_ERR(tty_dev);
 		goto err_release_data_interface;
-- 
2.37.0

-- 
Simon Arlott

Powered by blists - more mailing lists