linux-kernel - Re: [PATCHv2] usb: gadget: get rid of USB_GADGET_DUALSPEED and USB_GADGET

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.44L0.1108181623390.1628-100000@iolanthe.rowland.org>
Date:	Thu, 18 Aug 2011 16:30:14 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Michal Nazarewicz <mnazarewicz@...gle.com>
cc:	Sergei Shtylyov <sshtylyov@...sta.com>,
	Felipe Balbi <balbi@...com>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	Yang Rui Rui <ruirui.r.yang@...to.com>,
	Greg Kroah-Hartman <gregkh@...e.de>,
	<linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCHv2] usb: gadget: get rid of USB_GADGET_DUALSPEED and
 USB_GADGET_SUPERSPEED

On Thu, 18 Aug 2011, Michal Nazarewicz wrote:

> usb_add_function() is called before usb_composite_probe() which only then
> calls usb_gadget_probe_driver().
> 
> What's more, if I understand it correctly, comment in usb_add_function()
> says that we support all the speeds usb_composite_driver structure claims
> we do but then return different sets of functions depending on speed.  Ie.
> if a function is only full speed and host requests high speed,  
> configuration
> will lack that function.  The comment in question is:
> 
> 	/* We allow configurations that don't work at both speeds.
> 	 * If we run into a lowspeed Linux system, treat it the same
> 	 * as full speed ... it's the function drivers that will need
> 	 * to avoid bulk and ISO transfers. */
> 
> usb_add_function() then proceeds to set config->fullspeed,  
> config->highspeed
> and config->superspeed depending on what descriptors function provides.

Have you misinterpreted that comment?  To me, it seems to be saying 
that the gadget should allow connections that are slower than the 
maximum supported speed.  But that's not what I'm talking about -- I'm 
suggesting that the gadget needs to avoid running faster than the 
maximum supported speed.

> Those are later used in config_desc() where it iterates over all
> configurations checking which have functions supporting given speed:
> 
> 	list_for_each_entry(c, &cdev->configs, list) {
> 		switch (speed) {
> 		case USB_SPEED_SUPER: if (!c->superspeed) continue; break;
> 		case USB_SPEED_HIGH: if (!c->highspeed) continue; break;
> 		default: if (!c->fullspeed) continue;
> 		}
> 		if (w_value == 0)
> 			return config_buf(c, speed, cdev->req->buf, type);
> 		w_value--;
> 	}
> 
> config_buf() at the same time, iterates over all functions and skips the  
> ones
> that do not provide descriptors for given speed:
> 
> 	list_for_each_entry(f, &config->functions, list) {
> 		switch (speed) {
> 		case USB_SPEED_SUPER: descriptors = f->ss_descriptors; break;
> 		case USB_SPEED_HIGH: descriptors = f->hs_descriptors; break;
> 		default: descriptors = f->descriptors;
> 		}
> 		if (!descriptors)
> 			continue;
> [...]
> 	}
> 
> So I think that your suggestion to use composite_driver.speed =
> driver->max_speed was by all means correct.

Okay, that line was probably all right, but it seems that something
else needs to be changed.  In a composite gadget, if none of the
function drivers support SuperSpeed then we don't want
composite_driver.speed to be set to USB_SPEED_SUPER.  It would be 
foolish to allow the UDC to connect at a speed which would make the 
gadget useless.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/