| 
      
      
      From: Stephan M. <ste...@we...> - 2006-08-20 17:27:06
       | 
| Try the latest code from the CVS. I modified the driver's set=5Fconfiguratio= n() function. The function's bahaviour is now compatible to the Linux version.= Stephan > I've spotted what to me seems a pretty major difference between libusb > and libusb-win32. In the call to set=5Fconfiguration, libusb takes the > actual configuration value, i.e. the value specified in the > configuration descriptor, whereas libusb-win32 takes the index value + > 1. >=20 > This means that if you have a quirky device which has one configuration > called configuration 2, then with libusb you would use > set=5Fconfiguration(2) to set it, whereas on libusb-win32 you would use > set=5Fconfiguration(1). This is a bit troublesome. >=20 > We have been hit by this since a device we are using makes the same > error in reverse, it returns the config descriptor by value instead of > index, so when windows initially asks for the 0th config descriptor, > unless there is a bogus descriptor in the device called configuration 0 > (which of course doesn't make sense since config 0 is the unconfigured > state), the device returns an error. A workaround is to provide 2 config= > descriptors, one bogus one called 0 and the real one called 1. This is > fine on linux since you can select config 1 by calling > set=5Fconfiguration(1), but on windows you have to call > set=5Fconfiguration(2). If someone decided not to have incrementing > configuration values you'd be sunk with libusb-win32 at the moment. >=20 > Anyway, for the moment I'm just happy that I've worked out what's going > on and that I can select my configuration using 2. >=20 > Dan >=20 > ------------------------------------------------------------------------= - > Using Tomcat but need to do more=3F Need to support web services, security= =3F > Get stuff done quickly with pre-integrated technology to make your job e= asier > Download IBM WebSphere Application Server v.1.0.1 based on Apache Geroni= mo > http://sel.as-us.falkag.net/sel=3Fcmd=3Dlnk&kid=3D120709&bid=3D263057&dat=3D121642= > =5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F > Libusb-win32-devel mailing list > Lib...@li... > https://lists.sourceforge.net/lists/listinfo/libusb-win32-devel =5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F=5F= Erweitern Sie FreeMail zu einem noch leistungsst=E4rkeren E-Mail-Postfach! =09 Mehr Infos unter http://freemail.web.de/home/landingpad/=3Fmc=3D021131 | 
| 
      
      
      From: Dan E. <Dan...@ne...> - 2006-08-21 10:24:53
       | 
| Thanks, I'll try that. Dan. Stephan Meyer wrote: > Try the latest code from the CVS. I modified the driver's > set_configuration() function. The function's bahaviour is now > compatible to the Linux version. =20 >=20 > Stephan >=20 >=20 >> I've spotted what to me seems a pretty major difference between >> libusb and libusb-win32. In the call to set_configuration, libusb >> takes the actual configuration value, i.e. the value specified in the >> configuration descriptor, whereas libusb-win32 takes the index value >> + 1.=20 >>=20 >> This means that if you have a quirky device which has one >> configuration called configuration 2, then with libusb you would use >> set_configuration(2) to set it, whereas on libusb-win32 you would use >> set_configuration(1). This is a bit troublesome. >>=20 >> We have been hit by this since a device we are using makes the same >> error in reverse, it returns the config descriptor by value instead >> of index, so when windows initially asks for the 0th config >> descriptor, unless there is a bogus descriptor in the device called >> configuration 0 (which of course doesn't make sense since config 0 >> is the unconfigured state), the device returns an error. A >> workaround is to provide 2 config descriptors, one bogus one called >> 0 and the real one called 1. This is fine on linux since you can >> select config 1 by calling set_configuration(1), but on windows you >> have to call set_configuration(2). If someone decided not to have >> incrementing configuration values you'd be sunk with libusb-win32 at >> the moment.=20 >>=20 >> Anyway, for the moment I'm just happy that I've worked out what's >> going on and that I can select my configuration using 2. >>=20 >> Dan >>=20 >> = ---------------------------------------------------------------------- >> --- Using Tomcat but need to do more? Need to support web services, >> security? Get stuff done quickly with pre-integrated technology to >> make your job easier Download IBM WebSphere Application Server >> v.1.0.1 based on Apache Geronimo >> = http://sel.as-us.falkag.net/sel?cmd=3Dlnk&kid=3D120709&bid=3D263057&dat=3D= 1216 >> 42 _______________________________________________=20 >> Libusb-win32-devel mailing list >> Lib...@li... >> https://lists.sourceforge.net/lists/listinfo/libusb-win32-devel >=20 >=20 > = _________________________________________________________________________= _ > Erweitern Sie FreeMail zu einem noch leistungsst=E4rkeren > E-Mail-Postfach!=20 > Mehr Infos unter http://freemail.web.de/home/landingpad/?mc=3D021131 >=20 >=20 > = -------------------------------------------------------------------------= > Using Tomcat but need to do more? Need to support web services, > security?=20 > Get stuff done quickly with pre-integrated technology to make your > job easier Download IBM WebSphere Application Server v.1.0.1 based on > Apache Geronimo > = http://sel.as-us.falkag.net/sel?cmd=3Dlnk&kid=3D120709&bid=3D263057&dat=3D= 121642 > _______________________________________________ =20 > Libusb-win32-devel mailing list > Lib...@li... > https://lists.sourceforge.net/lists/listinfo/libusb-win32-devel | 
| 
      
      
      From: Dan E. <Dan...@ne...> - 2006-08-21 14:11:32
       | 
| I've just spotted a problem which probably explains quite a bit of strange behaviour which has been seen: When a device is unplugged, a subsequent call to usb_find_devices will free the struct usb_device, even though some other part of the software may have a handle open on the device, with the handle structure holding a pointer of the (now invalid) device. Subsequent calls through the API using the handle will now use the freed memory which may now contain something completely different, and cause very strange behaviour, rather than just producing the correct error that there is no device present. I think the struct usb_device objects need to contain a reference count of how many times they've been opened, and they should only be immediately deleted in usb_find_devices if the reference count is zero, otherwise the object needs flagging as deleted. Furthermore, when a handle is closed, it should check if the device object has been marked as deleted and deleted it at that point, and any attempts to use a handle containing a deleted object should fail (i.e. there needs to be a check any time a handle is used that the object is not marked deleted). Not deleting the struct usb_device is a rather horrible workaround, at least then the memory won't have been corrupted and behaviour should be less unexpected. I'm not sure how badly this bug affects the linux version of the library, but they seem closer to releasing the new API which doesn't seem to suffer from this problem. Dan. | 
| 
      
      
      From: Graeme G. <gr...@ar...> - 2006-08-21 14:31:28
       | 
| Dan Ellis wrote: > I've just spotted a problem which probably explains quite a bit of > strange behaviour which has been seen: > > When a device is unplugged, a subsequent call to usb_find_devices will > free the struct usb_device, even though some other part of the software > may have a handle open on the device, with the handle structure holding > a pointer of the (now invalid) device. This might explain why merely listing available USB devices corrupts and breaks an existing Libusb-win32 USB connection running in another process... Graeme Gill. | 
| 
      
      
      From: Dan E. <Dan...@ne...> - 2006-08-22 14:28:10
       | 
| Graeme Gill wrote: > Dan Ellis wrote: >=20 >> I've just spotted a problem which probably explains quite a bit of >> strange behaviour which has been seen: >>=20 >> When a device is unplugged, a subsequent call to usb_find_devices >> will free the struct usb_device, even though some other part of the >> software may have a handle open on the device, with the handle >> structure holding a pointer of the (now invalid) device. >=20 > This might explain why merely listing available USB devices corrupts > and breaks an existing Libusb-win32 USB connection running in another > process... =20 Maybe, but I thought that each process connected to a a DLL created a new set of static variables? Certainly it can be a problem if more than one thread lists available devices, or one thread lists the devices while another one (or ones) use it. Dan. | 
| 
      
      
      From: Dan E. <Dan...@ne...> - 2006-08-23 10:54:08
       | 
| Graeme Gill wrote: > Dan Ellis wrote: >=20 >> I've just spotted a problem which probably explains quite a bit of >> strange behaviour which has been seen: >>=20 >> When a device is unplugged, a subsequent call to usb_find_devices >> will free the struct usb_device, even though some other part of the >> software may have a handle open on the device, with the handle >> structure holding a pointer of the (now invalid) device. >=20 > This might explain why merely listing available USB devices corrupts > and breaks an existing Libusb-win32 USB connection running in another > process... =20 I've looked at this further, and I'm seriously doubting my initial suspicions. They were raised by colleagues who had seen heap corruption in the new code for putting children onto the virtual hub. I can't see anywhere that that the presence of the device structure in the handle structure would cause a problem. Dan. | 
| 
      
      
      From: Dan E. <Dan...@ne...> - 2006-08-23 13:25:27
       | 
| Ok, the culprit is finally tracked down, it was nothing to do with lifetimes of struct usb_devices, but in usb_os_determine_children which was only fairly recently added. When the number of children is counted, the number is reset from the previous occasion, and eventually wraps resulting in a malloc of 0, but still an attempt to write something in the location allocated which causes heap corruption. This was not causing a problem until the memory leak of virtual hubs was fixed because the new virtual hubs were always zeroed. Dan | 
| 
      
      
      From: Dan E. <Dan...@ne...> - 2006-08-23 15:12:03
       | 
| There's a small memory leak in usb_control_msg, in the case that
usb_io_sync fails and it's an OUT, then the malloced block doesn't get
freed:
  /* out request? */
  if(!(requesttype & USB_ENDPOINT_IN))
    {
      if(!(out =3D malloc(sizeof(libusb_request) + size)))
        {
          usb_error("usb_control_msg: memory allocation failed");=20
          return -ENOMEM;
        }
     =20
      memcpy(out, &req, sizeof(libusb_request));
      memcpy((char *)out + sizeof(libusb_request), bytes, size);
      out_size =3D sizeof(libusb_request) + size;
    }
  if(!usb_io_sync(dev->impl_info, code, out, out_size, in, in_size,
&read))
    {
      usb_error("usb_control_msg: sending control message failed, "
                "win error: %s", usb_win_error_to_string());
      return -usb_win_error_to_errno();
    }
  /* out request? */
  if(!(requesttype & USB_ENDPOINT_IN))
    {
      free(out);
      return size;
    }
  else
    return read;
Dan.
 | 
| 
      
      
      From: Adam K. <akr...@ro...> - 2006-08-23 15:42:16
       | 
| On Wed, Aug 23, 2006 at 04:11:55PM +0100, Dan Ellis wrote: > There's a small memory leak in usb_control_msg, in the case that > usb_io_sync fails and it's an OUT, then the malloced block doesn't get > freed: Hi, Dan, Thanks for posting about this bug and others. Have you considered posting patches to fix the bugs? That way myself and other users could try them out and Stephan, who I am sure is very busy, could review and apply the changes without having to implement them. --Adam | 
| 
      
      
      From: Dan E. <Dan...@ne...> - 2006-08-24 08:39:41
       | 
| Adam Kropelin wrote:
> On Wed, Aug 23, 2006 at 04:11:55PM +0100, Dan Ellis wrote:
>> There's a small memory leak in usb_control_msg, in the case that
>> usb_io_sync fails and it's an OUT, then the malloced block doesn't
>> get freed:
>=20
> Hi, Dan,
>=20
> Thanks for posting about this bug and others. Have you considered
> posting patches to fix the bugs? That way myself and other users
> could try them out and Stephan, who I am sure is very busy, could
> review and apply the changes without having to implement them.  =20
Good idea, here are the fixes for the memory leak and the corruption.
The diff :
Index: src/windows.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- windows.c	2006/08/08 16:16:03	1.52
+++ windows.c	2006/08/20 17:00:38	(working copy)
@@ -784,6 +784,11 @@
     {
       usb_error("usb_control_msg: sending control message failed, "
                 "win error: %s", usb_win_error_to_string());
+     =20
+      if(!(requesttype & USB_ENDPOINT_IN))
+	{
+	free(out);
+	}
       return -usb_win_error_to_errno();
     }
=20
@@ -1102,6 +1107,7 @@
           free(bus->root_dev->children);
         }
=20
+      bus->root_dev->num_children =3D 0;
       for(dev =3D bus->devices; dev; dev =3D dev->next)
         bus->root_dev->num_children++;
=20
 |