Hi,
On 12 November 2016 at 11:17, Marek Vasut <ma...@denx.de> wrote:
> On 11/12/2016 07:10 PM, Anatolij Gustschin wrote:
>> On Sat, 12 Nov 2016 10:36:42 +0100
>> Marek Vasut ma...@denx.de wrote:
>> ...
>>>> udev = dev_get_parent_priv(child);
>>>> + if (!udev)
>>>> + continue;
>>>
>>> I don't quite understand the problem from the patch description, but
>>> shouldn't all the return values from dev_get_parent_priv() be checked
>>> this way , not just these two ?
>>
>> The problem is that when dereferencing NULL udev we later access
>> some random address (e.g. when accessing dev->dev->parent in
>> usb_show_tree_graph()). dev->dev pointer is random DRAM data there,
>> when dereferencing it, data abort happens when random address
>> is outside of valid address range.
>
> I mean, I understand that udev can be NULL and we don't check it. But is
> udev == NULL an expected possibility ? And if so, when does such thing
> happen ?
>
>> Probably we should check elsewhere, at least where it might
>> return NULL.
>
> OK
>
>>>
>>> Why does dev_get_parent_priv() return NULL here ?
>>
>> it returns NULL because the dev->parent_priv is not allocated for
>> usb_mass_storage.lun0 device. I do not know the reason why.
>
> That's probably what needs to be fixed , no ?
Yes that seems wrong. There is a check immediately before this:
if (!device_active(child))
continue;
If the device is active, it must have been probed and so must have parent data.
See:
UCLASS_DRIVER(usb) = {
...
.per_child_auto_alloc_size = sizeof(struct usb_device),
Try 'dm tree' to see what the bad USB device is.
>
> Also, we should most likely check all the return values of
> dev_get_parent_priv() in cmd/usb.c, not just these two.
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
