Hi,
this series fixes hub_configure() error handling that causes hub->ports[i]
NULL pointer dereferences.
Changes in v3:
Added Acked-by from Alan Stern.
Changes in v2:
After review by Alan Stern. The new label has been placed in other place
to avoid changes in existing code. Third patch "usb: don't use bNbrPorts
after initialization" has been added.
The first version has been posted here:
https://lkml.org/lkml/2013/8/20/469
Krzysiek
Krzysztof Mazur (3):
usb: fix cleanup after failure in hub_configure()
usb: fail on usb_hub_create_port_device() errors
usb: don't use bNbrPorts after initialization
drivers/usb/core/hub.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
--
1.8.4.rc4.527.g303b16c
If the hub_configure() fails after setting the hdev->maxchild
the hub->ports might be NULL or point to uninitialized kzallocated
memory causing NULL pointer dereference in hub_quiesce() during cleanup.
Now after such error the hdev->maxchild is set to 0 to avoid cleanup
of uninitialized ports.
Signed-off-by: Krzysztof Mazur <[email protected]>
Acked-by: Alan Stern <[email protected]>
---
drivers/usb/core/hub.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 558313d..affed11 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1568,6 +1568,7 @@ static int hub_configure(struct usb_hub *hub,
return 0;
fail:
+ hdev->maxchild = 0;
dev_err (hub_dev, "config failed, %s (err %d)\n",
message, ret);
/* hub_disconnect() frees urb and descriptor */
--
1.8.4.rc4.527.g303b16c
After successful initialization hub->descriptor->bNbrPorts and
hub->hdev->maxchild are equal, but using hub->hdev->maxchild is
preferred because that value is explicitly used for initialization
of hub->ports[].
Signed-off-by: Krzysztof Mazur <[email protected]>
Acked-by: Alan Stern <[email protected]>
---
drivers/usb/core/hub.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 292ffa8..86e353f 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -451,7 +451,7 @@ static void led_work (struct work_struct *work)
if (hdev->state != USB_STATE_CONFIGURED || hub->quiescing)
return;
- for (i = 0; i < hub->descriptor->bNbrPorts; i++) {
+ for (i = 0; i < hdev->maxchild; i++) {
unsigned selector, mode;
/* 30%-50% duty cycle */
@@ -500,7 +500,7 @@ static void led_work (struct work_struct *work)
}
if (!changed && blinkenlights) {
cursor++;
- cursor %= hub->descriptor->bNbrPorts;
+ cursor %= hdev->maxchild;
set_port_led(hub, cursor + 1, HUB_LED_GREEN);
hub->indicator[cursor] = INDICATOR_CYCLE;
changed++;
@@ -826,7 +826,7 @@ static unsigned hub_power_on(struct usb_hub *hub, bool do_delay)
else
dev_dbg(hub->intfdev, "trying to enable port power on "
"non-switchable hub\n");
- for (port1 = 1; port1 <= hub->descriptor->bNbrPorts; port1++)
+ for (port1 = 1; port1 <= hub->hdev->maxchild; port1++)
if (hub->ports[port1 - 1]->power_is_on)
set_port_feature(hub->hdev, port1, USB_PORT_FEAT_POWER);
else
@@ -4655,9 +4655,7 @@ static void hub_events(void)
hub_dev = hub->intfdev;
intf = to_usb_interface(hub_dev);
dev_dbg(hub_dev, "state %d ports %d chg %04x evt %04x\n",
- hdev->state, hub->descriptor
- ? hub->descriptor->bNbrPorts
- : 0,
+ hdev->state, hdev->maxchild,
/* NOTE: expects max 15 ports... */
(u16) hub->change_bits[0],
(u16) hub->event_bits[0]);
@@ -4702,7 +4700,7 @@ static void hub_events(void)
}
/* deal with port status changes */
- for (i = 1; i <= hub->descriptor->bNbrPorts; i++) {
+ for (i = 1; i <= hdev->maxchild; i++) {
if (test_bit(i, hub->busy_bits))
continue;
connect_change = test_bit(i, hub->change_bits);
--
1.8.4.rc4.527.g303b16c
Ignoring usb_hub_create_port_device() errors cause later NULL pointer
deference when uninitialized hub->ports[i] entries are dereferenced
after port memory allocation error.
Signed-off-by: Krzysztof Mazur <[email protected]>
Acked-by: Alan Stern <[email protected]>
---
drivers/usb/core/hub.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index affed11..292ffa8 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1557,10 +1557,15 @@ static int hub_configure(struct usb_hub *hub,
if (hub->has_indicators && blinkenlights)
hub->indicator [0] = INDICATOR_CYCLE;
- for (i = 0; i < hdev->maxchild; i++)
- if (usb_hub_create_port_device(hub, i + 1) < 0)
+ for (i = 0; i < hdev->maxchild; i++) {
+ ret = usb_hub_create_port_device(hub, i + 1);
+ if (ret < 0) {
dev_err(hub->intfdev,
"couldn't create port%d device.\n", i + 1);
+ hdev->maxchild = i;
+ goto fail_keep_maxchild;
+ }
+ }
usb_hub_adjust_deviceremovable(hdev, hub->descriptor);
@@ -1569,6 +1574,7 @@ static int hub_configure(struct usb_hub *hub,
fail:
hdev->maxchild = 0;
+fail_keep_maxchild:
dev_err (hub_dev, "config failed, %s (err %d)\n",
message, ret);
/* hub_disconnect() frees urb and descriptor */
--
1.8.4.rc4.527.g303b16c