Hello, Oliver, all.
Unfortunately, there are at least 4 different concerns about this patch:
1) "!dev->actconfig->interface[ifnum]" won't catch a case where the value is not
NULL but some garbage. This way the system may crash with "general protection fault"
later. I've hit this case during my tests.
2) "(ifnum >= USB_MAXINTERFACES)" does not cover all the error conditions. "ifnum"
should be compared to "dev->actconfig->desc.bNumInterfaces". I.e. compared to the
number of "struct usb_interface" kzalloc()-ed, not to USB_MAXINTERFACES.
3) If returned like this ("return -ENODEV;") there is a "usb_device" leak. There is
usb_get_dev() in the first line of usbvision_probe() but no usb_put_dev() on this
failure path. Commit "afd270d1" addresses exactly this case in other failure paths.
4) There is a bug of the same type several lines below with number of endpoints. The
code is accessing hard-coded second endpoint ("interface->endpoint[1].desc") which
may not exist. It would be great to handle this in the same patch too.
I've just posted my version of the patch for the same issue to the list (subject:
"[media] usbvision: fix crash on detecting device with invalid configuration",
Message-Id: <1447696511-17704-2-git-send-email-vdronov@xxxxxxxxxx>). I believe it
resolves all the above concerns.
Best regards,
Vladis Dronov | Red Hat, Inc.| Product Security Engineer
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
