[RESEND,v4] usb: core: stop USB enumeration if too many retries
Commit Message
When a broken USB accessory connects to a USB host, usbcore might
keep doing enumeration retries. If the host has a watchdog mechanism,
the kernel panic will happen on the host.
This patch provides an attribute early_stop to limit the numbers of retries
for each port of a hub. If a port was marked with early_stop attribute,
unsuccessful connection attempts will fail quickly. In addition, if an
early_stop port has failed to initialize, it will ignore all future
connection events until early_stop attribute is clear.
Signed-off-by: Ray Chi <raychi@google.com>
---
Documentation/ABI/testing/sysfs-bus-usb | 11 +++++
drivers/usb/core/hub.c | 59 +++++++++++++++++++++++++
drivers/usb/core/hub.h | 4 ++
drivers/usb/core/port.c | 27 +++++++++++
4 files changed, 101 insertions(+)
Comments
On Fri, Oct 28, 2022 at 05:01:16PM +0800, Ray Chi wrote:
> When a broken USB accessory connects to a USB host, usbcore might
> keep doing enumeration retries. If the host has a watchdog mechanism,
> the kernel panic will happen on the host.
>
> This patch provides an attribute early_stop to limit the numbers of retries
> for each port of a hub. If a port was marked with early_stop attribute,
> unsuccessful connection attempts will fail quickly. In addition, if an
> early_stop port has failed to initialize, it will ignore all future
> connection events until early_stop attribute is clear.
>
> Signed-off-by: Ray Chi <raychi@google.com>
> ---
> Documentation/ABI/testing/sysfs-bus-usb | 11 +++++
> drivers/usb/core/hub.c | 59 +++++++++++++++++++++++++
> drivers/usb/core/hub.h | 4 ++
> drivers/usb/core/port.c | 27 +++++++++++
> 4 files changed, 101 insertions(+)
>
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- This looks like a new version of a previously submitted patch, but you
did not list below the --- line any changes from the previous version.
Please read the section entitled "The canonical patch format" in the
kernel file, Documentation/SubmittingPatches for what needs to be done
here to properly describe this.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
@@ -264,6 +264,17 @@ Description:
attached to the port will not be detected, initialized,
or enumerated.
+What: /sys/bus/usb/devices/.../<hub_interface>/port<X>/early_stop
+Date: Sep 2022
+Contact: Ray Chi <raychi@google.com>
+Description:
+ Some USB hosts have some watchdog mechanisms so that the device
+ may enter ramdump if it takes a long time during port initialization.
+ This attribute allows each port just has two attempts so that the
+ port initialization will be failed quickly. In addition, if a port
+ which is marked with early_stop has failed to initialize, it will ignore
+ all future connections until this attribute is clear.
+
What: /sys/bus/usb/devices/.../power/usb2_lpm_l1_timeout
Date: May 2013
Contact: Mathias Nyman <mathias.nyman@linux.intel.com>
@@ -3081,6 +3081,48 @@ static int hub_port_reset(struct usb_hub *hub, int port1,
return status;
}
+/*
+ * hub_port_stop_enumerate - stop USB enumeration or ignore port events
+ * @hub: target hub
+ * @port1: port num of the port
+ * @retries: port retries number of hub_port_init()
+ *
+ * Return:
+ * true: ignore port actions/events or give up connection attempts.
+ * false: keep original behavior.
+ *
+ * This function will be based on retries to check whether the port which is
+ * marked with early_stop attribute would stop enumeration or ignore events.
+ *
+ * Note:
+ * This function didn't change anything if early_stop is not set, and it will
+ * prevent all connection attempts when early_stop is set and the attempts of
+ * the port are more than 1.
+ */
+static bool hub_port_stop_enumerate(struct usb_hub *hub, int port1, int retries)
+{
+ struct usb_port *port_dev = hub->ports[port1 - 1];
+
+ if (port_dev->early_stop) {
+ if (port_dev->ignore_event)
+ return true;
+
+ /*
+ * We want unsuccessful attempts to fail quickly.
+ * Since some devices may need one failure during
+ * port initialization, we allow two tries but no
+ * more.
+ */
+ if (retries < 1)
+ return false;
+
+ port_dev->ignore_event = 1;
+ } else
+ port_dev->ignore_event = 0;
+
+ return port_dev->ignore_event;
+}
+
/* Check if a port is power on */
int usb_port_is_power_on(struct usb_hub *hub, unsigned int portstatus)
{
@@ -4855,6 +4897,11 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
buf->bMaxPacketSize0;
kfree(buf);
+ if (r < 0 && hub_port_stop_enumerate(hub, port1, retries)) {
+ retval = r;
+ goto fail;
+ }
+
retval = hub_port_reset(hub, port1, udev, delay, false);
if (retval < 0) /* error or disconnect */
goto fail;
@@ -5387,6 +5434,9 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus,
if ((status == -ENOTCONN) || (status == -ENOTSUPP))
break;
+ if (hub_port_stop_enumerate(hub, port1, i))
+ break;
+
/* When halfway through our retry count, power-cycle the port */
if (i == (PORT_INIT_TRIES - 1) / 2) {
dev_info(&port_dev->dev, "attempt power cycle\n");
@@ -5614,6 +5664,10 @@ static void port_event(struct usb_hub *hub, int port1)
if (!pm_runtime_active(&port_dev->dev))
return;
+ /* skip port actions if ignore_event is true*/
+ if (hub_port_stop_enumerate(hub, port1, 0))
+ return;
+
if (hub_handle_remote_wakeup(hub, port1, portstatus, portchange))
connect_change = 1;
@@ -5934,6 +5988,9 @@ static int usb_reset_and_verify_device(struct usb_device *udev)
ret = hub_port_init(parent_hub, udev, port1, i);
if (ret >= 0 || ret == -ENOTCONN || ret == -ENODEV)
break;
+
+ if (hub_port_stop_enumerate(parent_hub, port1, i))
+ goto stop_enumerate;
}
mutex_unlock(hcd->address0_mutex);
@@ -6022,6 +6079,8 @@ static int usb_reset_and_verify_device(struct usb_device *udev)
udev->bos = bos;
return 0;
+stop_enumerate:
+ mutex_unlock(hcd->address0_mutex);
re_enumerate:
usb_release_bos_descriptor(udev);
udev->bos = bos;
@@ -90,6 +90,8 @@ struct usb_hub {
* @is_superspeed cache super-speed status
* @usb3_lpm_u1_permit: whether USB3 U1 LPM is permitted.
* @usb3_lpm_u2_permit: whether USB3 U2 LPM is permitted.
+ * @early_stop: whether port initialization will be stopped earlier.
+ * @ignore_event: whether events of the port are ignored.
*/
struct usb_port {
struct usb_device *child;
@@ -103,6 +105,8 @@ struct usb_port {
u32 over_current_count;
u8 portnum;
u32 quirks;
+ unsigned int early_stop:1;
+ unsigned int ignore_event:1;
unsigned int is_superspeed:1;
unsigned int usb3_lpm_u1_permit:1;
unsigned int usb3_lpm_u2_permit:1;
@@ -17,6 +17,32 @@ static int usb_port_block_power_off;
static const struct attribute_group *port_dev_group[];
+static ssize_t early_stop_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct usb_port *port_dev = to_usb_port(dev);
+
+ return sysfs_emit(buf, "%s\n", port_dev->early_stop ? "yes" : "no");
+}
+
+static ssize_t early_stop_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct usb_port *port_dev = to_usb_port(dev);
+ bool value;
+
+ if (kstrtobool(buf, &value))
+ return -EINVAL;
+
+ if (value)
+ port_dev->early_stop = 1;
+ else
+ port_dev->early_stop = 0;
+
+ return count;
+}
+static DEVICE_ATTR_RW(early_stop);
+
static ssize_t disable_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -236,6 +262,7 @@ static struct attribute *port_dev_attrs[] = {
&dev_attr_quirks.attr,
&dev_attr_over_current_count.attr,
&dev_attr_disable.attr,
+ &dev_attr_early_stop.attr,
NULL,
};