From 07fdfc5e9f1c966be8722e8fa927e5ea140df5ce Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 18 Feb 2015 10:34:50 +0700 Subject: USB: serial: fix potential use-after-free after failed probe Fix return value in probe error path, which could end up returning success (0) on errors. This could in turn lead to use-after-free or double free (e.g. in port_remove) when the port device is removed. Fixes: c706ebdfc895 ("USB: usb-serial: call port_probe and port_remove at the right times") Cc: stable # v2.6.31 Signed-off-by: Johan Hovold Acked-by: Greg Kroah-Hartman --- drivers/usb/serial/bus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/serial/bus.c') diff --git a/drivers/usb/serial/bus.c b/drivers/usb/serial/bus.c index 9374bd2aba20..5d8d86666b90 100644 --- a/drivers/usb/serial/bus.c +++ b/drivers/usb/serial/bus.c @@ -75,7 +75,7 @@ static int usb_serial_device_probe(struct device *dev) retval = device_create_file(dev, &dev_attr_port_number); if (retval) { if (driver->port_remove) - retval = driver->port_remove(port); + driver->port_remove(port); goto exit_with_autopm; } -- cgit v1.2.3 From ca4383a3947a83286bc9b9c598a1f55e867871d7 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 18 Feb 2015 10:34:51 +0700 Subject: USB: serial: fix tty-device error handling at probe Add missing error handling when registering the tty device at port probe. This avoids trying to remove an uninitialised character device when the port device is removed. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Reported-by: Takashi Iwai Cc: stable # v2.6.12 Signed-off-by: Johan Hovold Acked-by: Greg Kroah-Hartman --- drivers/usb/serial/bus.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'drivers/usb/serial/bus.c') diff --git a/drivers/usb/serial/bus.c b/drivers/usb/serial/bus.c index 5d8d86666b90..6f91eb9ae81a 100644 --- a/drivers/usb/serial/bus.c +++ b/drivers/usb/serial/bus.c @@ -51,6 +51,7 @@ static int usb_serial_device_probe(struct device *dev) { struct usb_serial_driver *driver; struct usb_serial_port *port; + struct device *tty_dev; int retval = 0; int minor; @@ -80,7 +81,15 @@ static int usb_serial_device_probe(struct device *dev) } minor = port->minor; - tty_register_device(usb_serial_tty_driver, minor, dev); + tty_dev = tty_register_device(usb_serial_tty_driver, minor, dev); + if (IS_ERR(tty_dev)) { + retval = PTR_ERR(tty_dev); + device_remove_file(dev, &dev_attr_port_number); + if (driver->port_remove) + driver->port_remove(port); + goto exit_with_autopm; + } + dev_info(&port->serial->dev->dev, "%s converter now attached to ttyUSB%d\n", driver->description, minor); -- cgit v1.2.3 From 2deb96b5d4bb20a33bfaf80e30f38f3433653054 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 18 Feb 2015 10:34:52 +0700 Subject: USB: serial: fix port attribute-creation race Fix attribute-creation race with userspace by using the port device groups field to create the port attributes. Also use %u when printing the port number, which is unsigned, even though we do not currently support more than 128 ports per device. Reported-by: Takashi Iwai Signed-off-by: Johan Hovold Acked-by: Greg Kroah-Hartman --- drivers/usb/serial/bus.c | 19 ------------------- drivers/usb/serial/usb-serial.c | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 19 deletions(-) (limited to 'drivers/usb/serial/bus.c') diff --git a/drivers/usb/serial/bus.c b/drivers/usb/serial/bus.c index 6f91eb9ae81a..b53a28692226 100644 --- a/drivers/usb/serial/bus.c +++ b/drivers/usb/serial/bus.c @@ -38,15 +38,6 @@ static int usb_serial_device_match(struct device *dev, return 0; } -static ssize_t port_number_show(struct device *dev, - struct device_attribute *attr, char *buf) -{ - struct usb_serial_port *port = to_usb_serial_port(dev); - - return sprintf(buf, "%d\n", port->port_number); -} -static DEVICE_ATTR_RO(port_number); - static int usb_serial_device_probe(struct device *dev) { struct usb_serial_driver *driver; @@ -73,18 +64,10 @@ static int usb_serial_device_probe(struct device *dev) goto exit_with_autopm; } - retval = device_create_file(dev, &dev_attr_port_number); - if (retval) { - if (driver->port_remove) - driver->port_remove(port); - goto exit_with_autopm; - } - minor = port->minor; tty_dev = tty_register_device(usb_serial_tty_driver, minor, dev); if (IS_ERR(tty_dev)) { retval = PTR_ERR(tty_dev); - device_remove_file(dev, &dev_attr_port_number); if (driver->port_remove) driver->port_remove(port); goto exit_with_autopm; @@ -123,8 +106,6 @@ static int usb_serial_device_remove(struct device *dev) minor = port->minor; tty_unregister_device(usb_serial_tty_driver, minor); - device_remove_file(&port->dev, &dev_attr_port_number); - driver = port->serial->type; if (driver->port_remove) retval = driver->port_remove(port); diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c index 19842370a07f..529066bbc7e8 100644 --- a/drivers/usb/serial/usb-serial.c +++ b/drivers/usb/serial/usb-serial.c @@ -687,6 +687,21 @@ static void serial_port_dtr_rts(struct tty_port *port, int on) drv->dtr_rts(p, on); } +static ssize_t port_number_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct usb_serial_port *port = to_usb_serial_port(dev); + + return sprintf(buf, "%u\n", port->port_number); +} +static DEVICE_ATTR_RO(port_number); + +static struct attribute *usb_serial_port_attrs[] = { + &dev_attr_port_number.attr, + NULL +}; +ATTRIBUTE_GROUPS(usb_serial_port); + static const struct tty_port_operations serial_port_ops = { .carrier_raised = serial_port_carrier_raised, .dtr_rts = serial_port_dtr_rts, @@ -902,6 +917,7 @@ static int usb_serial_probe(struct usb_interface *interface, port->dev.driver = NULL; port->dev.bus = &usb_serial_bus_type; port->dev.release = &usb_serial_port_release; + port->dev.groups = usb_serial_port_groups; device_initialize(&port->dev); } -- cgit v1.2.3 From d6f7f41274b548435ab5de1041a492fc4a714196 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 18 Feb 2015 11:04:46 +0700 Subject: USB: serial: clean up bus probe error handling Clean up bus probe error handling by separating success and error paths. Signed-off-by: Johan Hovold --- drivers/usb/serial/bus.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) (limited to 'drivers/usb/serial/bus.c') diff --git a/drivers/usb/serial/bus.c b/drivers/usb/serial/bus.c index b53a28692226..8936a83c96cd 100644 --- a/drivers/usb/serial/bus.c +++ b/drivers/usb/serial/bus.c @@ -47,39 +47,42 @@ static int usb_serial_device_probe(struct device *dev) int minor; port = to_usb_serial_port(dev); - if (!port) { - retval = -ENODEV; - goto exit; - } + if (!port) + return -ENODEV; /* make sure suspend/resume doesn't race against port_probe */ retval = usb_autopm_get_interface(port->serial->interface); if (retval) - goto exit; + return retval; driver = port->serial->type; if (driver->port_probe) { retval = driver->port_probe(port); if (retval) - goto exit_with_autopm; + goto err_autopm_put; } minor = port->minor; tty_dev = tty_register_device(usb_serial_tty_driver, minor, dev); if (IS_ERR(tty_dev)) { retval = PTR_ERR(tty_dev); - if (driver->port_remove) - driver->port_remove(port); - goto exit_with_autopm; + goto err_port_remove; } + usb_autopm_put_interface(port->serial->interface); + dev_info(&port->serial->dev->dev, "%s converter now attached to ttyUSB%d\n", driver->description, minor); -exit_with_autopm: + return 0; + +err_port_remove: + if (driver->port_remove) + driver->port_remove(port); +err_autopm_put: usb_autopm_put_interface(port->serial->interface); -exit: + return retval; } -- cgit v1.2.3