usb/80773: "usbd_get_string()" could have taken a length parameter

[Alfred Perlstein]

The following reply was made to PR usb/80773; it has been noted by GNATS.

From: Alfred Perlstein <alfred at freebsd.org>
To: "M. Warner Losh" <imp at bsdimp.com>
Cc: linimon at freebsd.org, bug-followup at freebsd.org, freebsd-usb at freebsd.org
Subject: Re: usb/80773: "usbd_get_string()" could have taken a length parameter
Date: Sat, 30 Jun 2007 18:53:03 -0700

 no reason not to supply a usbd_get_stringn function which when given
 '-1' trusts the device?
 
 * M. Warner Losh <imp at bsdimp.com> [070630 08:08] wrote:
 > Please find enclosed a patch for this.  I'm sitting on the fence as to
 > whether or not to commit it, since it is an api/abi change.
 > 
 > Warner
 
 > Index: if_cdce.c
 > ===================================================================
 > RCS file: /home/ncvs/src/sys/dev/usb/if_cdce.c,v
 > retrieving revision 1.24
 > diff -u -r1.24 if_cdce.c
 > --- if_cdce.c	23 Jun 2007 06:47:43 -0000	1.24
 > +++ if_cdce.c	30 Jun 2007 14:28:41 -0000
 > @@ -280,7 +280,8 @@
 >  
 >  	ue = (const usb_cdc_ethernet_descriptor_t *)usb_find_desc(dev,
 >  	    UDESC_INTERFACE, UDESCSUB_CDC_ENF);
 > -	if (!ue || usbd_get_string(dev, ue->iMacAddress, eaddr_str)) {
 > +	if (!ue || usbd_get_string(dev, ue->iMacAddress, eaddr_str,
 > +	    sizeof(eaddr_str))) {
 >  		/* Fake MAC address */
 >  		device_printf(sc->cdce_dev, "faking MAC address\n");
 >  		eaddr[0]= 0x2a;
 > Index: uhub.c
 > ===================================================================
 > RCS file: /home/ncvs/src/sys/dev/usb/uhub.c,v
 > retrieving revision 1.81
 > diff -u -r1.81 uhub.c
 > --- uhub.c	29 Jun 2007 20:34:42 -0000	1.81
 > +++ uhub.c	30 Jun 2007 14:28:41 -0000
 > @@ -655,7 +655,8 @@
 >  
 >  found_dev:
 >  	/* XXX can sleep */
 > -	(void)usbd_get_string(dev, dev->ddesc.iSerialNumber, &serial[0]);
 > +	(void)usbd_get_string(dev, dev->ddesc.iSerialNumber, serial,
 > +	    sizeof(serial));
 >  	if (dev->ifacenums == NULL) {
 >  		snprintf(buf, buflen, "vendor=0x%04x product=0x%04x "
 >  		    "devclass=0x%02x devsubclass=0x%02x "
 > Index: usb_subr.c
 > ===================================================================
 > RCS file: /home/ncvs/src/sys/dev/usb/usb_subr.c,v
 > retrieving revision 1.94
 > diff -u -r1.94 usb_subr.c
 > --- usb_subr.c	20 Jun 2007 05:10:54 -0000	1.94
 > +++ usb_subr.c	30 Jun 2007 14:28:42 -0000
 > @@ -213,12 +213,14 @@
 >  	}
 >  
 >  	if (usedev) {
 > -		if (usbd_get_string(dev, udd->iManufacturer, v))
 > +		if (usbd_get_string(dev, udd->iManufacturer, v,
 > +		    USB_MAX_STRING_LEN))
 >  			vendor = NULL;
 >  		else
 >  			vendor = v;
 >  		usbd_trim_spaces(vendor);
 > -		if (usbd_get_string(dev, udd->iProduct, p))
 > +		if (usbd_get_string(dev, udd->iProduct, p,
 > +		    USB_MAX_STRING_LEN))
 >  			product = NULL;
 >  		else
 >  			product = p;
 > Index: usbdi.c
 > ===================================================================
 > RCS file: /home/ncvs/src/sys/dev/usb/usbdi.c,v
 > retrieving revision 1.102
 > diff -u -r1.102 usbdi.c
 > --- usbdi.c	20 Jun 2007 05:10:54 -0000	1.102
 > +++ usbdi.c	30 Jun 2007 14:28:42 -0000
 > @@ -1310,7 +1310,7 @@
 >  }
 >  
 >  usbd_status
 > -usbd_get_string(usbd_device_handle dev, int si, char *buf)
 > +usbd_get_string(usbd_device_handle dev, int si, char *buf, size_t len)
 >  {
 >  	int swap = dev->quirks->uq_flags & UQ_SWAP_UNICODE;
 >  	usb_string_descriptor_t us;
 > @@ -1321,6 +1321,8 @@
 >  	int size;
 >  
 >  	buf[0] = '\0';
 > +	if (len == 0)
 > +		return (USBD_NORMAL_COMPLETION);
 >  	if (si == 0)
 >  		return (USBD_INVAL);
 >  	if (dev->quirks->uq_flags & UQ_NO_STRINGS)
 > @@ -1342,7 +1344,7 @@
 >  		return (err);
 >  	s = buf;
 >  	n = size / 2 - 1;
 > -	for (i = 0; i < n; i++) {
 > +	for (i = 0; i < n && i < len - 1; i++) {
 >  		c = UGETW(us.bString[i]);
 >  		/* Convert from Unicode, handle buggy strings. */
 >  		if ((c & 0xff00) == 0)
 > Index: usbdi.h
 > ===================================================================
 > RCS file: /home/ncvs/src/sys/dev/usb/usbdi.h,v
 > retrieving revision 1.62
 > diff -u -r1.62 usbdi.h
 > --- usbdi.h	12 Jun 2007 19:40:20 -0000	1.62
 > +++ usbdi.h	30 Jun 2007 14:28:42 -0000
 > @@ -173,7 +173,8 @@
 >  
 >  int usbd_ratecheck(struct timeval *last);
 >  
 > -usbd_status usbd_get_string(usbd_device_handle dev, int si, char *buf);
 > +usbd_status usbd_get_string(usbd_device_handle dev, int si, char *buf,
 > +    size_t len);
 >  
 >  /* An iterator for descriptors. */
 >  typedef struct {
 
 > _______________________________________________
 > freebsd-usb at freebsd.org mailing list
 > http://lists.freebsd.org/mailman/listinfo/freebsd-usb
 > To unsubscribe, send any mail to "freebsd-usb-unsubscribe at freebsd.org"
 
 
 -- 
 - Alfred Perlstein