Re: [PATCH] USB: core: of: Check device_node before parsing in usb_of_get_child_node()

From: Vaibhav Hiremath
Date: Tue Aug 16 2016 - 17:21:59 EST




On Monday 15 August 2016 06:33 PM, Peter Chen wrote:
On Mon, Aug 15, 2016 at 11:31:10AM -0700, Vaibhav Hiremath wrote:
In case of HUB devices connected to USB ports, we may not have DT
node representing it inside USB, and when devices connected to hub
gets enumerated, call to usb_of_get_child_node() leads to NULL pointer
dereference.

In the usecase we have, where EHCI port is connected to USB HUB
device, and downward ports of HUB are connected to further USB
devices. When those devices gets enumerated, in order,
1. USB HUB ->
-> Call to usb_of_get_child_node() is OK, as
parent->dev.of_node is pointing to host node.
2. Devices connected to downward port of USB HUB
-> Call to usb_of_get_child_node() leads to NULL
pointer dereference as parent->dev.of_node = NULL,
as USB HUB DTS node may be empty.

Fix this NULL pointer dereference by adding check for pointer
device_node inside usb_of_get_child_node() fn.

Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@xxxxxxxxxx>
---
Testing: I have build tested it against mainline.

drivers/usb/core/of.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/usb/core/of.c b/drivers/usb/core/of.c
index 2289700..dc667a3 100644
--- a/drivers/usb/core/of.c
+++ b/drivers/usb/core/of.c
@@ -34,6 +34,9 @@ struct device_node *usb_of_get_child_node(struct device_node *parent,
struct device_node *node;
u32 port;
+ if (!parent)
+ return NULL;
+
for_each_child_of_node(parent, node) {
if (!of_property_read_u32(node, "reg", &port)) {
if (port == portnum)
I am afraid I can't reproduce it, would you please show me your dump
when null pointer dereference occurs? From what I find the
__of_get_next_child checks null pointer for parent node.


Peter,
You are right, __of_get_next_child is taking care of this.

When I observed this issue with my setup [1], I only looked at changes in
the mainline for of.c and core/usb.c, did not see the anything.....

Anyways, for the record, we do not need this patch. Instead I need to backport
below commit from mainline to my kernel base.

commit 43cb43678705e39b175b325f17938295996aefc7
Author: Florian Fainelli <f.fainelli@xxxxxxxxx>
Date: Wed May 28 10:39:02 2014 -0700

of: handle NULL node in next_child iterators

Add an early check for the node argument in __of_get_next_child and
of_get_next_available_child() to avoid dereferencing a NULL node pointer
a few lines after.


[1] Also I missed to mention about my kernel version, I am based on very
ancient kernel version (3.10). Do not ask me why, it is something out of my
control :)

--
Thanks,
Vaibhav