[libusb] Null-Pointer exception difference from 1.0.20 to 1.0.21

[Matthias Bußjäger]

Hello,

yesterday I updated from libusb 1.0.20 to libusb 1.0.21 and ran into a
NULL pointer exception.
I also pulled the up to date sources from github and compiled it to
check if the problem still exists - and it did.

The problem exists in function windows_get_config_descriptor_by_value,
but should already have been avoided with the checks in the function
cache_config_descriptors.
Unfortunately there seem to exist cases that need an additional check:
I added a NULL pointer check in static int
windows_get_config_descriptor_by_value and could access all the USB
devices like before with version 1.0.20.
The device leading to this strange behavior was an Evoluent
VerticalMouse. It worked (!), but there obviously was a problem with the
configuration descriptor for libusb and I couldn't get further
information than available in the device descriptor.
I then unplugged this device and inserted it again - and the problem was
gone. My added NULL pointer check wasn't necessary any more.
I also can't reproduce this problem again and therefore I currently
didn't make a pull request for the fix yet.

My question is now: Should I make a pull request for the additional check?
I would like to do that because I had the problem that version 1.0.20
was more stable than the current one. And I think it should not get more
unstable, also in case of device specific problems or similar. But
unfortunately I can't provide a guide to reproduce this problem.
I made some screenshots before the unplug that show some values while
debugging with visual studio, that's all I could provide.

Best regards,
Matthias

[Xiaofan Chen]

On Thu, Feb 23, 2017 at 7:41 PM, Matthias Bußjäger

Post by Matthias Bußjäger
Hello,
yesterday I updated from libusb 1.0.20 to libusb 1.0.21 and ran into a
NULL pointer exception.
I also pulled the up to date sources from github and compiled it to
check if the problem still exists - and it did.
...
My question is now: Should I make a pull request for the additional check?
I would like to do that because I had the problem that version 1.0.20
was more stable than the current one. And I think it should not get more
unstable, also in case of device specific problems or similar. But
unfortunately I can't provide a guide to reproduce this problem.
I made some screenshots before the unplug that show some values while
debugging with visual studio, that's all I could provide.

I think there is no problem to create a pull request.

[Matthias Bußjäger]

Hi Xiaofan and others,

I'm new to github and hope I did the pull request correctly...
I also have some new knowledge about the problem, because some PCs on
our side always show a similar and reproducible behavior.
So in summary there are now 2 cases I know where the Null-Pointer will
appear at the part of code of my pull-request (mbussjaeg:master):
1.) The unreproducible configuration descriptor problem as described
with my initial email (Evoluent mouse, but the device itself seems not
to be the problem)
2.) Some HCD hubs (this problem is reproducible on about 30% of the Host
PCs I tried here).

For case 2.) I can give further information if necessary; I'll try to
explain the basics here:
You can run into the problem if you get all USB devices with
libusb_get_device_list, afterwards for all devices request the current
configuration using libusb_get_active_config_descriptor.
This will include also Hubs etc.
All of the devices from libusb_get_device_list have been handled by the
function "init_device", but for HCD hubs there will only be a device
descriptor request and the cached data won't include the configuration
descriptor (--> the pointer will be NULL afterwards).
If you now request libusb to give you the active configuration
descriptor for a device, it will access the previously cached data:
And exactly here is a difference from libusb 1.0.20 to 1.0.21, the bug
was introduced with the implementation of the function
windows_get_config_descriptor_by_value one year ago.
1.0.20: Uses "windows_get_config_descriptor"; internally catched the
exception of non cached descriptor value with "if
((priv->config_descriptor == NULL) ||
(priv->config_descriptor[config_index] == NULL))"
1.0.21: Uses "windows_get_config_descriptor_by_value". This function now
correctly loops through all configurations but unfortunately misses a
similar check and therefore tries to access the NULL pointer of the
non-cached config descriptor --> crash.

Long explanation for this problem and only a small code adaption, but I
just hope to get the pull request merged as soon as possible now.

Thank you in advance,
Matthias

(Please don't hesitate to tell me if I could improve some flow with
github or this mailing list in the future)

Post by Xiaofan Chen
On Thu, Feb 23, 2017 at 7:41 PM, Matthias Bußjäger

Post by Matthias Bußjäger
Hello,
yesterday I updated from libusb 1.0.20 to libusb 1.0.21 and ran into a
NULL pointer exception.
I also pulled the up to date sources from github and compiled it to
check if the problem still exists - and it did.
...
My question is now: Should I make a pull request for the additional check?
I would like to do that because I had the problem that version 1.0.20
was more stable than the current one. And I think it should not get more
unstable, also in case of device specific problems or similar. But
unfortunately I can't provide a guide to reproduce this problem.
I made some screenshots before the unplug that show some values while
debugging with visual studio, that's all I could provide.

I think there is no problem to create a pull request.

[Matthias Bußjäger]

Ping.

(Just closed my initial pull request and opened a new one (#285) with
better comment and not from my master.)

Regards,
Matthias

Post by Matthias Bußjäger
Hi Xiaofan and others,
I'm new to github and hope I did the pull request correctly...
I also have some new knowledge about the problem, because some PCs on
our side always show a similar and reproducible behavior.
So in summary there are now 2 cases I know where the Null-Pointer will
1.) The unreproducible configuration descriptor problem as described
with my initial email (Evoluent mouse, but the device itself seems not
to be the problem)
2.) Some HCD hubs (this problem is reproducible on about 30% of the
Host PCs I tried here).
For case 2.) I can give further information if necessary; I'll try to
You can run into the problem if you get all USB devices with
libusb_get_device_list, afterwards for all devices request the current
configuration using libusb_get_active_config_descriptor.
This will include also Hubs etc.
All of the devices from libusb_get_device_list have been handled by
the function "init_device", but for HCD hubs there will only be a
device descriptor request and the cached data won't include the
configuration descriptor (--> the pointer will be NULL afterwards).
If you now request libusb to give you the active configuration
And exactly here is a difference from libusb 1.0.20 to 1.0.21, the bug
was introduced with the implementation of the function
windows_get_config_descriptor_by_value one year ago.
1.0.20: Uses "windows_get_config_descriptor"; internally catched the
exception of non cached descriptor value with "if
((priv->config_descriptor == NULL) ||
(priv->config_descriptor[config_index] == NULL))"
1.0.21: Uses "windows_get_config_descriptor_by_value". This function
now correctly loops through all configurations but unfortunately
misses a similar check and therefore tries to access the NULL pointer
of the non-cached config descriptor --> crash.
Long explanation for this problem and only a small code adaption, but
I just hope to get the pull request merged as soon as possible now.
Thank you in advance,
Matthias
(Please don't hesitate to tell me if I could improve some flow with
github or this mailing list in the future)

Post by Xiaofan Chen
On Thu, Feb 23, 2017 at 7:41 PM, Matthias Bußjäger

Post by Matthias Bußjäger
Hello,
yesterday I updated from libusb 1.0.20 to libusb 1.0.21 and ran into a
NULL pointer exception.
I also pulled the up to date sources from github and compiled it to
check if the problem still exists - and it did.
...
My question is now: Should I make a pull request for the additional check?
I would like to do that because I had the problem that version 1.0.20
was more stable than the current one. And I think it should not get more
unstable, also in case of device specific problems or similar. But
unfortunately I can't provide a guide to reproduce this problem.
I made some screenshots before the unplug that show some values while
debugging with visual studio, that's all I could provide.

I think there is no problem to create a pull request.