[PATCH] Fix for USB ugen panics

Hi

Attached is a patch which I believe fixes two issues found in the ugen driver, 
as described in these mails:

http://lists.freebsd.org/pipermail/freebsd-current/2003-November/015065.html
http://lists.freebsd.org/pipermail/freebsd-current/2003-December/016260.html

The first is caused by an unhandled attempt to set a USB device's 
configuration to number 0 (the unconfigurd state, USB_UNCONFIG_NO) and the 
second by an inconsistent state left after an error occurs in one of ugen's 
functions, since the introduction of devfs.

A breakdown of the patch and justification for each change is given below. 
Note that I've only been able to confirm the first bug has been fixed, as the 
second did not have an easily reproducable cause.

-	struct ugen_endpoint *sce;
-	u_int8_t niface, nendpt;
+	struct ugen_endpoint *sce, **sce_cache, ***sce_cache_arr;
+	u_int8_t niface, niface_cache, nendpt, *nendpt_cache;

Four variables are declared to facilitate the endpoint caching method 
described below.

-#if defined(__FreeBSD__)
-	ugen_destroy_devnodes(sc);
-#endif

This call is moved further down into the function. If any of the subsequent 
function calls fail and return(err) (this can happen during normal use in 
usbd_set_config_no(), for example), devfs will be left in an inconsistent 
state which will panic on the next attempt to ugen_destroy_devnodes.

-	/* Avoid setting the current value. */
-	if (usbd_get_config_descriptor(dev)->bConfigurationValue != configno) {
-		err = usbd_set_config_no(dev, configno, 1);
-		if (err)
-			return (err);
-	}

This code is moved further down into the function. We must cache the current 
set of endpoint descriptors before the call to usbd_set_config_no(), otherwise 
if the call succeeds we will be unable to clear the old set of endpoint 
descriptors. This is needed as a consequence of moving the 
ugen_destroy_devnodes() call below here.

-	memset(sc->sc_endpoints, 0, sizeof sc->sc_endpoints);

This is an unnecessary call now, as we clear each endpoint descriptor 
explicitly later.

+ [Large set of endpoint caching code]

This allocates and stores in a structure the set of endpoint descriptors so 
they can be freed after the ugen_destroy_devnodes call. I wish there were a 
cleaner way to achieve this, but the separation of devfs from internal usbd_* 
code makes this necessary.

-			return (err);
+			panic("ugen_set_config: can't obtain interface handle");

Leaving the function at this point will result in an inconsistent state. 
Better to panic now.

-			return (err);
+			panic("ugen_set_config: endpoint count failed");

Ditto.

+	if (configno != USB_UNCONFIG_NO)
+	{

Continuing at this point with a NULL dev->cdesc will result in a panic in 
usbd_interface_count. The endpoint regeneration code is skipped instead.

All the changes in the patch should be attributable to one of the explanations 
given above.

I welcome any comments on the patch, particularly on structure and the use of 
M_TEMP (I couldn't find documentation on the correct form of malloc for 
allocating temporary storage). :)

-- 
Cheers,
Jay

http://www.evilrealms.net/ - Systems Administrator & Developer
http://www.imperial.ac.uk/ - 3rd year CS student