Skip to contentSkip to conversation

hid2hci check usb_init return value

PATCH In review Khem Raj 3 participants 8 messages Dec 25, 2008 – Jan 6, 2009 Bluetooth drivers

lore
Share & tools
lore

Khem Raj

author
#0 lore raw

PATCH 1/1 hid2hci check usb_init return value

Hi

Working on a system which did not have a USB in it. I was getting a segfault with hid2hci. The problem tracked down was usb_init () was failing and we did not check for it and carried on with execution. As a result the next call to libusb failed because we passed in NULL for ctx because during usb_init () this should have been malloc'ed if all have gone correctly but in case of error usb_init () free's up the allocated memory and hence the segfault.

We should check for return value of usb_init () call and exit immediately if it fails. Tested on 4.24 release.

OK ?

Thanks

-Khem

# Signed-off-by: Khem Raj <[email protected]>
#
# Use the new usb1 API for usb_init() and check for fails from

# usb_init (). Currently we see a crash on a system which does
# not have USB because usb_init() fails and it cleans up all initialized
# data (e.g. ctx) which is used in subsequent calls to libusb
# We return immediately if usb_init() fails for some reason.
tools/hid2hci.c +4 -3
Index: bluez-4.24/tools/hid2hci.c
===================================================================
@@ -337,7 +337,7 @@
 int main(int argc, char *argv[])
 {
 	struct device_info dev[16];
-	int i, opt, num, quiet = 0, mode = HCI;
+	int i, ret, opt, num, quiet = 0, mode = HCI;
 
 	while ((opt = getopt_long(argc, argv, "+01qh", main_options, NULL)) != -1) {
 		switch (opt) {

@@ -361,8 +361,9 @@
 	argc -= optind;
 	argv += optind;
 	optind = 0;
-
-	usb_init();
+	ret = usb_init();
+	if (ret < 0)
+		return ret;
 
 	num = find_devices(mode, dev, sizeof(dev) / sizeof(dev[0]));
 	if (num <= 0) {
maintainer
#1 lore raw

Hi Khem,

Quoted 10 lines

Working on a system which did not have a USB in it. I was getting a segfault with hid2hci. The problem tracked down was usb_init () was failing and we did not check for it and carried on with execution. As a result the next call to libusb failed because we passed in NULL for ctx because during usb_init () this should have been malloc'ed if all have gone correctly but in case of error usb_init () free's up the allocated memory and hence the segfault.

We should check for return value of usb_init () call and exit immediately if it fails. Tested on 4.24 release.

the problem is that usb_init() doesn't return any errors on the versions that I have here:

ii  libusb-0.1-4   2:0.1.12-12    userspace USB programming library
ii  libusb-dev     2:0.1.12-12    userspace USB programming library

Applying your patch would break complication on multiple platforms.

Regards

Marcel

author
#2 lore raw
Quoted 13 lines · Marcel Holtmann

Hi Khem,

Quoted 10 lines

Working on a system which did not have a USB in it. I was getting a segfault with hid2hci. The problem tracked down was usb_init () was failing and we did not check for it and carried on with execution. As a result the next call to libusb failed because we passed in NULL for ctx because during usb_init () this should have been malloc'ed if all have gone correctly but in case of error usb_init () free's up the allocated memory and hence the segfault.

We should check for return value of usb_init () call and exit immediately if it fails. Tested on 4.24 release.

Quoted 7 lines

the problem is that usb_init() doesn't return any errors on the versions that I have here:

ii  libusb-0.1-4   2:0.1.12-12    userspace USB programming library
ii  libusb-dev     2:0.1.12-12    userspace USB programming library

Applying your patch would break complication on multiple platforms.

What you have is libusb-0.1 which has older API I suppose. There is libusb-compat which can be used as a wrapper over libusb1 to give libusb0 like interfaces.

Actually I was using libusb-1.0.0 API as mentioned here
http://libusb.sourceforge.net/api-1.0/group__lib.html
that also means that the patch I attached should have been using libusb_init() instead.

Here is updated patch

Thanks

-Khem

# Signed-off-by: Khem Raj <[email protected]>
#
# Use the new usb1 API for usb_init() and check for fails from

# usb_init (). Currently we see a crash on a system which does
# not have USB because usb_init() fails and it cleans up all initialized
# data (e.g. ctx) which is used in subsequent calls to libusb
# We return immediately if usb_init() fails for some reason.
tools/hid2hci.c +4 -3
Index: bluez-4.24/tools/hid2hci.c
===================================================================
@@ -337,7 +337,7 @@
 int main(int argc, char *argv[])
 {
 	struct device_info dev[16];
-	int i, opt, num, quiet = 0, mode = HCI;
+	int i, ret, opt, num, quiet = 0, mode = HCI;
 
 	while ((opt = getopt_long(argc, argv, "+01qh", main_options, NULL)) != -1) {
 		switch (opt) {

@@ -361,8 +361,9 @@
 	argc -= optind;
 	argv += optind;
 	optind = 0;
-
-	usb_init();
+	ret = libusb_init();
+	if (ret < 0)
+		return ret;
 
 	num = find_devices(mode, dev, sizeof(dev) / sizeof(dev[0]));
 	if (num <= 0) {
maintainer
#3 lore raw

Hi Khem,

Quoted 22 lines
Quoted 11 lines
Quoted 10 lines

Working on a system which did not have a USB in it. I was getting a segfault with hid2hci. The problem tracked down was usb_init () was failing and we did not check for it and carried on with execution. As a result the next call to libusb failed because we passed in NULL for ctx because during usb_init () this should have been malloc'ed if all have gone correctly but in case of error usb_init () free's up the allocated memory and hence the segfault.

We should check for return value of usb_init () call and exit immediately if it fails. Tested on 4.24 release.

Quoted 7 lines

the problem is that usb_init() doesn't return any errors on the versions that I have here:

ii  libusb-0.1-4   2:0.1.12-12    userspace USB programming library
ii  libusb-dev     2:0.1.12-12    userspace USB programming library

Applying your patch would break complication on multiple platforms.

What you have is libusb-0.1 which has older API I suppose. There is libusb-compat which can be used as a wrapper over libusb1 to give libusb0 like interfaces.

Quoted 6 lines

Actually I was using libusb-1.0.0 API as mentioned here
http://libusb.sourceforge.net/api-1.0/group__lib.html
that also means that the patch I attached should have been using libusb_init() instead.

Here is updated patch

this still breaks with a system that has libusb-0.1 installed like the current Ubuntu 8.10 does. So either we switch to libusb-1.0 completely or we leave it as it is.

Regards

Marcel

#4 lore raw
Quoted 8 lines · Marcel Holtmann
Quoted 8 lines

What you have is libusb-0.1 which has older API I suppose. There is libusb-compat which can be used as a wrapper over libusb1 to give libusb0 like interfaces.

Actually I was using libusb-1.0.0 API as mentioned here
http://libusb.sourceforge.net/api-1.0/group__lib.html
that also means that the patch I attached should have been using libusb_init() instead.

Quoted 7 lines
Quoted 2 lines

Here is updated patch

this still breaks with a system that has libusb-0.1 installed like the current Ubuntu 8.10 does. So either we switch to libusb-1.0 completely or we leave it as it is.

This looks like a job for autoconf?

maintainer
#5 lore raw

Hi David,

Quoted 12 lines
Quoted 12 lines
Quoted 11 lines

What you have is libusb-0.1 which has older API I suppose. There is libusb-compat which can be used as a wrapper over libusb1 to give libusb0 like interfaces.

Actually I was using libusb-1.0.0 API as mentioned here
http://libusb.sourceforge.net/api-1.0/group__lib.html
that also means that the patch I attached should have been using libusb_init() instead.

Here is updated patch

Quoted 6 lines
Quoted 4 lines

this still breaks with a system that has libusb-0.1 installed like the current Ubuntu 8.10 does. So either we switch to libusb-1.0 completely or we leave it as it is.

This looks like a job for autoconf?

it could be, but I am only doing either libusb-0.1 or libusb-1.0. We can have support for both, but these compile time options are a bad thing and I don't want them.

Regards

Marcel

author
#6 lore raw
In reply to Marcel Holtmann
Quoted 29 lines · Marcel Holtmann

Hi Khem,

Quoted 27 lines
Quoted 11 lines
Quoted 10 lines

Working on a system which did not have a USB in it. I was getting a segfault with hid2hci. The problem tracked down was usb_init () was failing and we did not check for it and carried on with execution. As a result the next call to libusb failed because we passed in NULL for ctx because during usb_init () this should have been malloc'ed if all have gone correctly but in case of error usb_init () free's up the allocated memory and hence the segfault.

We should check for return value of usb_init () call and exit immediately if it fails. Tested on 4.24 release.

Quoted 7 lines

the problem is that usb_init() doesn't return any errors on the versions that I have here:

ii  libusb-0.1-4   2:0.1.12-12    userspace USB programming library
ii  libusb-dev     2:0.1.12-12    userspace USB programming library

Applying your patch would break complication on multiple platforms.

What you have is libusb-0.1 which has older API I suppose. There is libusb-compat which can be used as a wrapper over libusb1 to give libusb0 like interfaces.

Actually I was using libusb-1.0.0 API as mentioned here
http://libusb.sourceforge.net/api-1.0/group__lib.html
that also means that the patch I attached should have been using libusb_init() instead.

Quoted 5 lines
Quoted 1 line

Here is updated patch

this still breaks with a system that has libusb-0.1 installed like the current Ubuntu 8.10 does. So either we switch to libusb-1.0 completely or we leave it as it is.

Moving forward depending on libusb-1.0 seems a better idea to me.

Thanks

-Khem

maintainer
#7 lore raw

Hi Khem,

Quoted 27 lines
Quoted 27 lines
Quoted 27 lines
Quoted 11 lines
Quoted 10 lines

Working on a system which did not have a USB in it. I was getting a segfault with hid2hci. The problem tracked down was usb_init () was failing and we did not check for it and carried on with execution. As a result the next call to libusb failed because we passed in NULL for ctx because during usb_init () this should have been malloc'ed if all have gone correctly but in case of error usb_init () free's up the allocated memory and hence the segfault.

We should check for return value of usb_init () call and exit immediately if it fails. Tested on 4.24 release.

Quoted 7 lines

the problem is that usb_init() doesn't return any errors on the versions that I have here:

ii  libusb-0.1-4   2:0.1.12-12    userspace USB programming library
ii  libusb-dev     2:0.1.12-12    userspace USB programming library

Applying your patch would break complication on multiple platforms.

What you have is libusb-0.1 which has older API I suppose. There is libusb-compat which can be used as a wrapper over libusb1 to give libusb0 like interfaces.

Actually I was using libusb-1.0.0 API as mentioned here
http://libusb.sourceforge.net/api-1.0/group__lib.html
that also means that the patch I attached should have been using libusb_init() instead.

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

Here is updated patch

this still breaks with a system that has libusb-0.1 installed like the current Ubuntu 8.10 does. So either we switch to libusb-1.0 completely or we leave it as it is.

Moving forward depending on libusb-1.0 seems a better idea to me.

feel free to submit patches for all utilities with USB support. Then we can talk about switching.

Regards

Marcel