On Thu, Oct 13, 2011 at 09:42:40PM -0400, Alan Stern wrote:
> On Thu, 13 Oct 2011, Paul Zimmerman wrote:
>
> > The latest USB-IF CV tester checks for a valid length for this
> > request.
>
> That's dumb. The BOT spec doesn't say anything about what a device
> should do if it receives a request with wLength > 1. Therefore it's
> wrong to say that a device is non-compliant with the spec if it replies
> to such requests.
>
> If anything, this should be a test of the host, not of the device.
> After all, it's the host's fault if wLength is set to the wrong value.
>
> Instead of changing this, I'd prefer to complain to the USB-IF.
It's the same with SetAddress command. A SetAddress() on Configured
state has unspecified answer, nevertheless USB-IF is very, very bad at
changing those Command Verifier tools and every time I have tried
reporting a bug (either on Windows Stack or Command Verifier Stack) they
always reply with the same stock answer: "Other devices have passed
certification on the same setup, so why can't you?"
That means the USB Certified sticker has nothing to do with being
compliant with the Spec, rather being compliant with Windows.
On top of all that, if we don't apply this there will be a big number of
Linux devices which will have to keep this patch on their own out of
tree queue just to pass certification.
Do we want that ? I think it's far better to apply this patch, but mark
it as host-side quirk or something. The host stack has quirks for
out-of-spec devices, why can't the device stack have quirk for
out-of-spec hosts ?
I would only ask Paul to change a small thing:
diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c
index 39ece40..1ab0101 100644
--- a/drivers/usb/gadget/file_storage.c
+++ b/drivers/usb/gadget/file_storage.c
@@ -866,6 +866,12 @@ static int class_setup_req(struct fsg_dev *fsg,
value = -EDOM;
break;
}
+ if (w_length != 1) {
+ DBG(fsg, "get max LUN with bogus length (%d)\n",
+ w_length);
+ value = -EMSGSIZE;
+ break;
+ }
VDBG(fsg, "get max LUN\n");
*(u8 *) req->buf = fsg->nluns - 1;
value = 1;
then it's easy to see if we have fallen into that case.
--
balbi
Attachment:
signature.asc
Description: Digital signature
