Re: Null ptr deref in core/hub.c

[Greg KH <gregkh@linuxfoundation.org>]

On Sat, Apr 24, 2021 at 02:08:45PM +0200, Felicitas Hetzelt wrote:
> Hello,
> 
> I triggered a few potential npds in core/hub.c. The bugs trigger
> reliably. Unfortunately I don't have a reproducer, though i tried my
> best to root-cause the bugs. I'm using my own emulated xhci host
> controller device and a slightly exotic kernel environment based on
> kernel version 5.10.0-rc6, so it might be that the bug is not
> trigger-able under normal conditions.
> 
> I was hoping you could maybe quickly determine whether this is a valid
> issue.
> 
> usb_hub_to_struct can return zero if hdev->actconfig->interface[0]->dev
> is NULL.

How can that be possible in a real system?

> 
> https://elixir.bootlin.com/linux/v4.9/source/drivers/usb/core/hub.c#L124
> 
> https://elixir.bootlin.com/linux/v4.9/source/include/linux/usb.h#L194

Note, 4.9 is VERY old :)

> This is the case when usb_probe_interface fails to probe the device
> driver (called via usb_set_configuration -> device_add -> ...)
> 
> https://elixir.bootlin.com/linux/v4.9/source/drivers/usb/core/driver.c#L372

And how can that happen in a real system?

> Then e.g. on a new invocation of hub_port_connect, the function tries to
> un-attach the previously attached devices (listed as port_dev->child)
> and calls recursively_mark_NOTATTACHED (via usb_set_device_state(udev,
> USB_STATE_NOTATTACHED), which in turn tries to get a pointer to the hub
> via usb_hub_to_struct_hub, which is NULL which leads to the crash.
> 
> https://elixir.bootlin.com/linux/v4.9/source/drivers/usb/core/hub.c#L4742
> 
> I feel like this should be caught in hub_port_connect (which would set
> port_dev->child = NULL, avoiding the later invocation of
> recursively_mark_NOTATTACHED), but the return value of usb_new_device is
> always valid (in fact usb_set_configuration can only return 0 once it
> gets to the calling add_device and probe).
> 
> https://elixir.bootlin.com/linux/v4.9/source/drivers/usb/core/hub.c#L4891
> 
> https://elixir.bootlin.com/linux/v4.9/source/drivers/usb/core/message.c#L1931
> 
> To fix this one could check whether the interface is actually properly
> setup instead of just checking status, or alternatively always check the
> return value of usb_hub_to_struct_hub on later invocations.

Patches are great to send, that's the best way to discuss issues that
you find.

thanks,

greg k-h