[PATCH] cmd: usb: Prevent reset in usb tree/info command

When DISTRO_DEFAULTS is not set, the default environment has bootcmd=bootflow and this will cause a UCLASS_BOOTDEV device to be added as sibling of those UCLASS_BLK devices in boot_targets, until boot succeeds from some device. If none succeeds, and usb is in boot_targets, and an usb storage device is plugged to some usb port at boot time, its UCLASS_MASS_STORAGE device will have a UCLASS_BOOTDEV device as child, besides a UCLASS_BLK child.
If once the boot fails the user enters at the U-Boot shell prompt:
usb info
or
usb tree
The code in cmd/usb.c will eventually recurse into the UCLASS_BOOTDEV and pass a null pointer to usb_device (because it has no parent_priv_). This causes a reset.
Fix it (twice) by checking for null parent_priv_ and adding UCLASS_BOOTDEV to the list of ignored class ids before the recursive call.
This prevents the current particular problem with UCLASS_BOOTDEV, even in case it ever gets some parent_priv_ struct which is not an usb_device, despite being the child of a usb_device->dev. And it also prevents possible future problems if other children are added to usb devices that don't have parent_priv_ because they are not part of the usb tree, just abstractions of functionality (like UCLASS_BLK and UCLASS_BOOTDEV are now).
Signed-off-by: Xavier Drudis Ferran xdrudis@tinet.cat ---
v2: added UCLASS_BOOTDEV check (discussion of v1 dried up without much evident consensus, so hopefully Simon Glass likes it better now) [ https://patchwork.ozlabs.org/project/uboot/patch/ZH39SVA1vbZR3+Gk@xdrudis.ti... ] ---
Apologies to the people in Cc: for resending this. I had a typo in the email list address.
--- cmd/usb.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/cmd/usb.c b/cmd/usb.c index 6193728384..23253f2223 100644 --- a/cmd/usb.c +++ b/cmd/usb.c @@ -421,7 +421,9 @@ static void usb_show_tree_graph(struct usb_device *dev, char *pre) * Ignore emulators and block child devices, we only want * real devices */ - if ((device_get_uclass_id(child) != UCLASS_USB_EMUL) && + if (udev && + (device_get_uclass_id(child) != UCLASS_BOOTDEV) && + (device_get_uclass_id(child) != UCLASS_USB_EMUL) && (device_get_uclass_id(child) != UCLASS_BLK)) { usb_show_tree_graph(udev, pre); pre[index] = 0; @@ -604,10 +606,12 @@ static void usb_show_info(struct usb_device *udev) child; device_find_next_child(&child)) { if (device_active(child) && + (device_get_uclass_id(child) != UCLASS_BOOTDEV) && (device_get_uclass_id(child) != UCLASS_USB_EMUL) && (device_get_uclass_id(child) != UCLASS_BLK)) { udev = dev_get_parent_priv(child); - usb_show_info(udev); + if (udev) + usb_show_info(udev); } } }

On 6/19/23 12:26, Xavier Drudis Ferran wrote:
When DISTRO_DEFAULTS is not set, the default environment has bootcmd=bootflow and this will cause a UCLASS_BOOTDEV device to be added as sibling of those UCLASS_BLK devices in boot_targets, until boot succeeds from some device. If none succeeds, and usb is in boot_targets, and an usb storage device is plugged to some usb port at boot time, its UCLASS_MASS_STORAGE device will have a UCLASS_BOOTDEV device as child, besides a UCLASS_BLK child.
If once the boot fails the user enters at the U-Boot shell prompt:
usb info
or
usb tree
The code in cmd/usb.c will eventually recurse into the UCLASS_BOOTDEV and pass a null pointer to usb_device (because it has no parent_priv_). This causes a reset.
Fix it (twice) by checking for null parent_priv_ and adding UCLASS_BOOTDEV to the list of ignored class ids before the recursive call.
This prevents the current particular problem with UCLASS_BOOTDEV, even in case it ever gets some parent_priv_ struct which is not an usb_device, despite being the child of a usb_device->dev. And it also prevents possible future problems if other children are added to usb devices that don't have parent_priv_ because they are not part of the usb tree, just abstractions of functionality (like UCLASS_BLK and UCLASS_BOOTDEV are now).
Signed-off-by: Xavier Drudis Ferran xdrudis@tinet.cat
v2: added UCLASS_BOOTDEV check (discussion of v1 dried up without much evident consensus, so hopefully Simon Glass likes it better now) [ https://patchwork.ozlabs.org/project/uboot/patch/ZH39SVA1vbZR3+Gk@xdrudis.ti... ]
Apologies to the people in Cc: for resending this. I had a typo in the email list address.
git send-email -v2 would help
Also you can add a CC list into the commit message below --- , then git send-email will automatically pick the CC list up. To get a list of people to CC , use get-maintainer script.
participants (2)
-
Marek Vasut
-
Xavier Drudis Ferran