* [PATCH] enable usb control message with class specific request @ 2011-09-22 10:10 Matthias Dellweg 2011-09-22 15:12 ` Alan Stern 0 siblings, 1 reply; 11+ messages in thread From: Matthias Dellweg @ 2011-09-22 10:10 UTC (permalink / raw) To: Greg Kroah-Hartman, Vasiliy Kulikov, Michal Sojka, Arnd Bergmann, linux-usb, linux-kernel Hi! Usb devio assumes that the wIndex in every control message apart from those flagged as USB_TYPE_VENDOR holds the number of the Interface being addressed. This is for example not true for the class specific request GET_DEVICE_ID in the printer class: "The high-byte of the wIndex field is used to specify the zero-based interface index. The low-byte of the wIndex field is used to specify the zero-based alternate setting." [1] In this special case it misinterpretes the alternate setting 1 for the interface and tries to claim a nonexisting one. Therefor you won't get the printers name. The patch below is a minimal approach to fix this. Maybe it should be extended to USB_TYPE_RESERVED. Maybe there should be an extended test that knows something about specific classes. What do you think? regards Matthias [1] http://www.usb.org/developers/devclass_docs/usbprint11.pdf >From 724e3b5e8782a584a95d05bb2f44e59743ed3a72 Mon Sep 17 00:00:00 2001 From: Matthias Dellweg <2500@gmx.de> Date: Wed, 21 Sep 2011 21:28:18 +0200 Subject: [PATCH] drivers/usb/core/devio.c: Relax test in check_ctrlrecip The generic test for the interface is not valid when the request type is class specific. Signed-off-by: Matthias Dellweg <2500@gmx.de> --- drivers/usb/core/devio.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 37518df..4e78768 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -615,7 +615,8 @@ static int check_ctrlrecip(struct dev_state *ps, unsigned int requesttype, && ps->dev->state != USB_STATE_ADDRESS && ps->dev->state != USB_STATE_CONFIGURED) return -EHOSTUNREACH; - if (USB_TYPE_VENDOR == (USB_TYPE_MASK & requesttype)) + if ((USB_TYPE_VENDOR == (USB_TYPE_MASK & requesttype)) + || (USB_TYPE_CLASS == (USB_TYPE_MASK & requesttype))) return 0; index &= 0xff; -- 1.7.6.3 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] enable usb control message with class specific request 2011-09-22 10:10 [PATCH] enable usb control message with class specific request Matthias Dellweg @ 2011-09-22 15:12 ` Alan Stern 2011-09-22 21:56 ` Matthias Dellweg 0 siblings, 1 reply; 11+ messages in thread From: Alan Stern @ 2011-09-22 15:12 UTC (permalink / raw) To: Matthias Dellweg Cc: Greg Kroah-Hartman, Vasiliy Kulikov, Michal Sojka, Arnd Bergmann, linux-usb, linux-kernel On Thu, 22 Sep 2011, Matthias Dellweg wrote: > Hi! > Usb devio assumes that the wIndex in every control message apart from > those flagged as USB_TYPE_VENDOR holds the number of the Interface > being addressed. This is for example not true for the class specific > request GET_DEVICE_ID in the printer class: > > "The high-byte of the wIndex field is used to specify the zero-based > interface index. The low-byte of the wIndex field is used to specify > the zero-based alternate setting." [1] > > In this special case it misinterpretes the alternate setting 1 for the > interface and tries to claim a nonexisting one. Therefor you won't get > the printers name. > > The patch below is a minimal approach to fix this. Maybe it should be > extended to USB_TYPE_RESERVED. Maybe there should be an extended test > that knows something about specific classes. > > What do you think? > regards Matthias > > [1] http://www.usb.org/developers/devclass_docs/usbprint11.pdf In this case, it appears that the printer class specification contradicts the USB-2.0 specification. Section 9.3.1 says (referring to the low-order five bits of bmRequestType): Requests may be directed to the device, an interface on the device, or a specific endpoint on a device. This field also specifies the intended recipient of the request. When an interface or endpoint is specified, the wIndex field identifies the interface or endpoint. And Figure 9-3 shows that when wIndex is used to specify an interface, the interface number belongs in the low-order byte, not the high-order byte. I don't think it's safe to relax the test the way you have suggested. There are too many other class-specific requests that must be prevented. Maybe an exception could be added for this one particular case. Besides, you don't want to remove the test entirely -- you want to use the high-order byte of wIndex instead of the low-order byte. The printer spec really is spectacularly bad in this respect. What happens if the printer is a composite device, and the other interface uses the same bmRequestType and bRequest values for its own class-specific purpose, but uses the low-order byte of wIndex to indicate the interface number (as it should). Then the printer wouldn't know which interface was supposed to respond to the message! Alan Stern ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] enable usb control message with class specific request 2011-09-22 15:12 ` Alan Stern @ 2011-09-22 21:56 ` Matthias Dellweg 2011-09-23 7:05 ` Matthias Dellweg 0 siblings, 1 reply; 11+ messages in thread From: Matthias Dellweg @ 2011-09-22 21:56 UTC (permalink / raw) To: Alan Stern Cc: Greg Kroah-Hartman, Vasiliy Kulikov, Michal Sojka, Arnd Bergmann, linux-usb, linux-kernel Am Thu, 22 Sep 2011 11:12:51 -0400 (EDT) schrieb Alan Stern <stern@rowland.harvard.edu>: > On Thu, 22 Sep 2011, Matthias Dellweg wrote: > > > Hi! > > Usb devio assumes that the wIndex in every control message apart > > from those flagged as USB_TYPE_VENDOR holds the number of the > > Interface being addressed. This is for example not true for the > > class specific request GET_DEVICE_ID in the printer class: > > > > "The high-byte of the wIndex field is used to specify the zero-based > > interface index. The low-byte of the wIndex field is used to specify > > the zero-based alternate setting." [1] > > > > In this special case it misinterpretes the alternate setting 1 for > > the interface and tries to claim a nonexisting one. Therefor you > > won't get the printers name. > > > > The patch below is a minimal approach to fix this. Maybe it should > > be extended to USB_TYPE_RESERVED. Maybe there should be an extended > > test that knows something about specific classes. > > > > What do you think? > > regards Matthias > > > > [1] http://www.usb.org/developers/devclass_docs/usbprint11.pdf > > In this case, it appears that the printer class specification > contradicts the USB-2.0 specification. Section 9.3.1 says (referring > to the low-order five bits of bmRequestType): > > Requests may be directed to the device, an interface on the > device, or a specific endpoint on a device. This field also > specifies the intended recipient of the request. When an > interface or endpoint is specified, the wIndex field > identifies the interface or endpoint. > > And Figure 9-3 shows that when wIndex is used to specify an > interface, the interface number belongs in the low-order byte, not > the high-order byte. > > I don't think it's safe to relax the test the way you have suggested. > There are too many other class-specific requests that must be > prevented. Maybe an exception could be added for this one particular > case. Besides, you don't want to remove the test entirely -- you > want to use the high-order byte of wIndex instead of the low-order > byte. > > The printer spec really is spectacularly bad in this respect. What > happens if the printer is a composite device, and the other interface > uses the same bmRequestType and bRequest values for its own > class-specific purpose, but uses the low-order byte of wIndex to > indicate the interface number (as it should). Then the printer > wouldn't know which interface was supposed to respond to the message! > > Alan Stern OK, let's assume this is the only exception in the specs. Do you think the test should look like this: >From 6514baecca5e193b78e1f3343585c5c9a458a625 Mon Sep 17 00:00:00 2001 From: Matthias Dellweg <2500@gmx.de> Date: Thu, 22 Sep 2011 23:50:35 +0200 Subject: [PATCH] drivers/usb/core/devio.c: Check for printer class specific request Signed-off-by: Matthias Dellweg <2500@gmx.de> --- drivers/usb/core/devio.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 37518df..e3e60f7 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -607,7 +607,7 @@ static int findintfep(struct usb_device *dev, unsigned int ep) } static int check_ctrlrecip(struct dev_state *ps, unsigned int requesttype, - unsigned int index) + unsigned int request, unsigned int index) { int ret = 0; @@ -618,6 +618,12 @@ static int check_ctrlrecip(struct dev_state *ps, unsigned int requesttype, if (USB_TYPE_VENDOR == (USB_TYPE_MASK & requesttype)) return 0; + /* check for a very special case in the printer class specification */ + if ((requesttype == 0xa1) && (request == 0x00) + && (usb_find_alt_setting(ps->dev->actconfig, index >> 8, index & 0xff) + ->desc.bInterfaceClass == USB_CLASS_PRINTER)) + index >>= 8; + index &= 0xff; switch (requesttype & USB_RECIP_MASK) { case USB_RECIP_ENDPOINT: @@ -770,7 +776,7 @@ static int proc_control(struct dev_state *ps, void __user *arg) if (copy_from_user(&ctrl, arg, sizeof(ctrl))) return -EFAULT; - ret = check_ctrlrecip(ps, ctrl.bRequestType, ctrl.wIndex); + ret = check_ctrlrecip(ps, ctrl.bRequestType, ctrl.bRequest, ctrl.wIndex); if (ret) return ret; wLength = ctrl.wLength; /* To suppress 64k PAGE_SIZE warning */ @@ -1100,7 +1106,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, kfree(dr); return -EINVAL; } - ret = check_ctrlrecip(ps, dr->bRequestType, + ret = check_ctrlrecip(ps, dr->bRequestType, dr->bRequest, le16_to_cpup(&dr->wIndex)); if (ret) { kfree(dr); -- 1.7.6.3 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] enable usb control message with class specific request 2011-09-22 21:56 ` Matthias Dellweg @ 2011-09-23 7:05 ` Matthias Dellweg 2011-09-23 16:16 ` Alan Stern 0 siblings, 1 reply; 11+ messages in thread From: Matthias Dellweg @ 2011-09-23 7:05 UTC (permalink / raw) To: Alan Stern Cc: Greg Kroah-Hartman, Vasiliy Kulikov, Michal Sojka, Arnd Bergmann, linux-usb, linux-kernel Am Thu, 22 Sep 2011 23:56:56 +0200 schrieb Matthias Dellweg <2500@gmx.de>: > + if ((requesttype == 0xa1) && (request == 0x00) > + && (usb_find_alt_setting(ps->dev->actconfig, index >> 8, > index & 0xff) > + ->desc.bInterfaceClass == USB_CLASS_PRINTER)) > + index >>= 8; I just woke up and realised this is a very bad idea if there is no such setting... So here's the next version I'd like to propose for comments: diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 37518df..d2efc97 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -607,9 +607,10 @@ static int findintfep(struct usb_device *dev, unsigned int ep) } static int check_ctrlrecip(struct dev_state *ps, unsigned int requesttype, - unsigned int index) + unsigned int request, unsigned int index) { int ret = 0; + struct usb_host_interface *alt_setting; if (ps->dev->state != USB_STATE_UNAUTHENTICATED && ps->dev->state != USB_STATE_ADDRESS @@ -618,6 +619,15 @@ static int check_ctrlrecip(struct dev_state *ps, unsigned int requesttype, if (USB_TYPE_VENDOR == (USB_TYPE_MASK & requesttype)) return 0; + /* check for the special corner case 'get_device_id' in the printer class specification, + * where wIndex is (configuration << 8 | altsetting) */ + if ((requesttype == 0xa1) && (request == 0x00)) + { + alt_setting = usb_find_alt_setting(ps->dev->actconfig, index >> 8, index & 0xff); + if (alt_setting && (alt_setting->desc.bInterfaceClass == USB_CLASS_PRINTER)) + index >>= 8; + } + index &= 0xff; switch (requesttype & USB_RECIP_MASK) { case USB_RECIP_ENDPOINT: @@ -770,7 +780,7 @@ static int proc_control(struct dev_state *ps, void __user *arg) if (copy_from_user(&ctrl, arg, sizeof(ctrl))) return -EFAULT; - ret = check_ctrlrecip(ps, ctrl.bRequestType, ctrl.wIndex); + ret = check_ctrlrecip(ps, ctrl.bRequestType, ctrl.bRequest, ctrl.wIndex); if (ret) return ret; wLength = ctrl.wLength; /* To suppress 64k PAGE_SIZE warning */ @@ -1100,7 +1110,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, kfree(dr); return -EINVAL; } - ret = check_ctrlrecip(ps, dr->bRequestType, + ret = check_ctrlrecip(ps, dr->bRequestType, dr->bRequest, le16_to_cpup(&dr->wIndex)); if (ret) { kfree(dr); ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] enable usb control message with class specific request 2011-09-23 7:05 ` Matthias Dellweg @ 2011-09-23 16:16 ` Alan Stern 2011-09-23 17:57 ` Matthias Dellweg 0 siblings, 1 reply; 11+ messages in thread From: Alan Stern @ 2011-09-23 16:16 UTC (permalink / raw) To: Matthias Dellweg Cc: Greg Kroah-Hartman, Vasiliy Kulikov, Michal Sojka, Arnd Bergmann, linux-usb, linux-kernel On Fri, 23 Sep 2011, Matthias Dellweg wrote: > Am Thu, 22 Sep 2011 23:56:56 +0200 > schrieb Matthias Dellweg <2500@gmx.de>: > > > + if ((requesttype == 0xa1) && (request == 0x00) > > + && (usb_find_alt_setting(ps->dev->actconfig, index >> 8, > > index & 0xff) > > + ->desc.bInterfaceClass == USB_CLASS_PRINTER)) > > + index >>= 8; > > I just woke up and realised this is a very bad idea if there is no such > setting... > So here's the next version I'd like to propose for comments: This is basically okay. But your mailer has mangled the whitespace. > diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c > index 37518df..d2efc97 100644 > --- a/drivers/usb/core/devio.c > +++ b/drivers/usb/core/devio.c > @@ -607,9 +607,10 @@ static int findintfep(struct usb_device *dev, unsigned int ep) > } > > static int check_ctrlrecip(struct dev_state *ps, unsigned int requesttype, > - unsigned int index) > + unsigned int request, unsigned int index) > { > int ret = 0; > + struct usb_host_interface *alt_setting; > > if (ps->dev->state != USB_STATE_UNAUTHENTICATED > && ps->dev->state != USB_STATE_ADDRESS > @@ -618,6 +619,15 @@ static int check_ctrlrecip(struct dev_state *ps, unsigned int requesttype, > if (USB_TYPE_VENDOR == (USB_TYPE_MASK & requesttype)) > return 0; > > + /* check for the special corner case 'get_device_id' in the printer class specification, > + * where wIndex is (configuration << 8 | altsetting) */ Here and below, you should fit lines into 80 columns. And the closing */ should be on a separate line (many people like to put the opening /* on a separate line too). Also, the high byte of wIndex is interface, not configuration. > + if ((requesttype == 0xa1) && (request == 0x00)) Nested parens not needed. And the request value should be given in decimal, like it is in the spec. > + { The open paren should be at the end of the "if" line, not on a line of its own. > + alt_setting = usb_find_alt_setting(ps->dev->actconfig, index >> 8, index & 0xff); > + if (alt_setting && (alt_setting->desc.bInterfaceClass == USB_CLASS_PRINTER)) > + index >>= 8; > + } > + > index &= 0xff; > switch (requesttype & USB_RECIP_MASK) { > case USB_RECIP_ENDPOINT: > @@ -770,7 +780,7 @@ static int proc_control(struct dev_state *ps, void __user *arg) > > if (copy_from_user(&ctrl, arg, sizeof(ctrl))) > return -EFAULT; > - ret = check_ctrlrecip(ps, ctrl.bRequestType, ctrl.wIndex); > + ret = check_ctrlrecip(ps, ctrl.bRequestType, ctrl.bRequest, ctrl.wIndex); > if (ret) > return ret; > wLength = ctrl.wLength; /* To suppress 64k PAGE_SIZE warning */ > @@ -1100,7 +1110,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, > kfree(dr); > return -EINVAL; > } > - ret = check_ctrlrecip(ps, dr->bRequestType, > + ret = check_ctrlrecip(ps, dr->bRequestType, dr->bRequest, > le16_to_cpup(&dr->wIndex)); > if (ret) { > kfree(dr); Alan Stern ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] enable usb control message with class specific request 2011-09-23 16:16 ` Alan Stern @ 2011-09-23 17:57 ` Matthias Dellweg 2011-09-23 18:31 ` Alan Stern 0 siblings, 1 reply; 11+ messages in thread From: Matthias Dellweg @ 2011-09-23 17:57 UTC (permalink / raw) To: Alan Stern Cc: Greg Kroah-Hartman, Vasiliy Kulikov, Michal Sojka, Arnd Bergmann, linux-usb, linux-kernel Am Fri, 23 Sep 2011 12:16:40 -0400 (EDT) schrieb Alan Stern <stern@rowland.harvard.edu>: > This is basically okay. But your mailer has mangled the whitespace. > [...] > Alan Stern OK, hoping that my mailer doesn't get wild again, this is the polished version following your suggestions. Thanks for your help! Matthias Dellweg >From b7c4b6f5780296fd0d53d66e08d5bce0dc450b7f Mon Sep 17 00:00:00 2001 From: Matthias Dellweg <2500@gmx.de> Date: Fri, 23 Sep 2011 19:32:39 +0200 Subject: [PATCH] drivers/usb/core/devio.c: Check for printer class specific request Signed-off-by: Matthias Dellweg <2500@gmx.de> --- drivers/usb/core/devio.c | 20 +++++++++++++++++--- 1 files changed, 17 insertions(+), 3 deletions(-) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 37518df..e4175cc 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -607,9 +607,10 @@ static int findintfep(struct usb_device *dev, unsigned int ep) } static int check_ctrlrecip(struct dev_state *ps, unsigned int requesttype, - unsigned int index) + unsigned int request, unsigned int index) { int ret = 0; + struct usb_host_interface *alt_setting; if (ps->dev->state != USB_STATE_UNAUTHENTICATED && ps->dev->state != USB_STATE_ADDRESS @@ -618,6 +619,18 @@ static int check_ctrlrecip(struct dev_state *ps, unsigned int requesttype, if (USB_TYPE_VENDOR == (USB_TYPE_MASK & requesttype)) return 0; + /* + * check for the special corner case 'get_device_id' in the printer + * class specification, where wIndex is (interface << 8 | altsetting) + */ + if (requesttype == 0xa1 && request == 0) { + alt_setting = usb_find_alt_setting(ps->dev->actconfig, + index >> 8, index & 0xff); + if (alt_setting + && alt_setting->desc.bInterfaceClass == USB_CLASS_PRINTER) + index >>= 8; + } + index &= 0xff; switch (requesttype & USB_RECIP_MASK) { case USB_RECIP_ENDPOINT: @@ -770,7 +783,8 @@ static int proc_control(struct dev_state *ps, void __user *arg) if (copy_from_user(&ctrl, arg, sizeof(ctrl))) return -EFAULT; - ret = check_ctrlrecip(ps, ctrl.bRequestType, ctrl.wIndex); + ret = check_ctrlrecip(ps, ctrl.bRequestType, ctrl.bRequest, + ctrl.wIndex); if (ret) return ret; wLength = ctrl.wLength; /* To suppress 64k PAGE_SIZE warning */ @@ -1100,7 +1114,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, kfree(dr); return -EINVAL; } - ret = check_ctrlrecip(ps, dr->bRequestType, + ret = check_ctrlrecip(ps, dr->bRequestType, dr->bRequest, le16_to_cpup(&dr->wIndex)); if (ret) { kfree(dr); -- 1.7.6.3 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] enable usb control message with class specific request 2011-09-23 17:57 ` Matthias Dellweg @ 2011-09-23 18:31 ` Alan Stern 2011-09-25 12:46 ` Matthias Dellweg 0 siblings, 1 reply; 11+ messages in thread From: Alan Stern @ 2011-09-23 18:31 UTC (permalink / raw) To: Matthias Dellweg Cc: Greg Kroah-Hartman, Vasiliy Kulikov, Michal Sojka, Arnd Bergmann, linux-usb, linux-kernel On Fri, 23 Sep 2011, Matthias Dellweg wrote: > Am Fri, 23 Sep 2011 12:16:40 -0400 (EDT) > schrieb Alan Stern <stern@rowland.harvard.edu>: > > This is basically okay. But your mailer has mangled the whitespace. > > [...] > > Alan Stern > > OK, hoping that my mailer doesn't get wild again, this is the polished version > following your suggestions. > Thanks for your help! > Matthias Dellweg > > From b7c4b6f5780296fd0d53d66e08d5bce0dc450b7f Mon Sep 17 00:00:00 2001 > From: Matthias Dellweg <2500@gmx.de> > Date: Fri, 23 Sep 2011 19:32:39 +0200 > Subject: [PATCH] drivers/usb/core/devio.c: Check for printer class specific > request > > Signed-off-by: Matthias Dellweg <2500@gmx.de> > --- > drivers/usb/core/devio.c | 20 +++++++++++++++++--- > 1 files changed, 17 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c > index 37518df..e4175cc 100644 > --- a/drivers/usb/core/devio.c > +++ b/drivers/usb/core/devio.c > @@ -607,9 +607,10 @@ static int findintfep(struct usb_device *dev, unsigned int ep) > } > > static int check_ctrlrecip(struct dev_state *ps, unsigned int requesttype, > - unsigned int index) > + unsigned int request, unsigned int index) > { > int ret = 0; > + struct usb_host_interface *alt_setting; > > if (ps->dev->state != USB_STATE_UNAUTHENTICATED > && ps->dev->state != USB_STATE_ADDRESS > @@ -618,6 +619,18 @@ static int check_ctrlrecip(struct dev_state *ps, unsigned int requesttype, > if (USB_TYPE_VENDOR == (USB_TYPE_MASK & requesttype)) > return 0; > > + /* > + * check for the special corner case 'get_device_id' in the printer > + * class specification, where wIndex is (interface << 8 | altsetting) > + */ > + if (requesttype == 0xa1 && request == 0) { > + alt_setting = usb_find_alt_setting(ps->dev->actconfig, > + index >> 8, index & 0xff); > + if (alt_setting > + && alt_setting->desc.bInterfaceClass == USB_CLASS_PRINTER) > + index >>= 8; > + } > + > index &= 0xff; > switch (requesttype & USB_RECIP_MASK) { > case USB_RECIP_ENDPOINT: > @@ -770,7 +783,8 @@ static int proc_control(struct dev_state *ps, void __user *arg) > > if (copy_from_user(&ctrl, arg, sizeof(ctrl))) > return -EFAULT; > - ret = check_ctrlrecip(ps, ctrl.bRequestType, ctrl.wIndex); > + ret = check_ctrlrecip(ps, ctrl.bRequestType, ctrl.bRequest, > + ctrl.wIndex); > if (ret) > return ret; > wLength = ctrl.wLength; /* To suppress 64k PAGE_SIZE warning */ > @@ -1100,7 +1114,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, > kfree(dr); > return -EINVAL; > } > - ret = check_ctrlrecip(ps, dr->bRequestType, > + ret = check_ctrlrecip(ps, dr->bRequestType, dr->bRequest, > le16_to_cpup(&dr->wIndex)); > if (ret) { > kfree(dr); > This looks fine to me. Greg may ask you for a non-empty patch description when you submit it to him. Acked-by: Alan Stern <stern@rowland.harvard.edu> Alan Stern ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] enable usb control message with class specific request 2011-09-23 18:31 ` Alan Stern @ 2011-09-25 12:46 ` Matthias Dellweg 2011-09-26 22:34 ` [PATCH] usb/core/devio.c: Check for printer " Greg KH 0 siblings, 1 reply; 11+ messages in thread From: Matthias Dellweg @ 2011-09-25 12:46 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: Alan Stern, linux-usb, linux-kernel Am Fri, 23 Sep 2011 14:31:40 -0400 (EDT) schrieb Alan Stern <stern@rowland.harvard.edu>: > On Fri, 23 Sep 2011, Matthias Dellweg wrote: > > > Am Fri, 23 Sep 2011 12:16:40 -0400 (EDT) > > schrieb Alan Stern <stern@rowland.harvard.edu>: > > > This is basically okay. But your mailer has mangled the > > > whitespace. [...] > > > Alan Stern > > > > OK, hoping that my mailer doesn't get wild again, this is the > > polished version following your suggestions. > > Thanks for your help! > > Matthias Dellweg > > > > From b7c4b6f5780296fd0d53d66e08d5bce0dc450b7f Mon Sep 17 00:00:00 > > 2001 From: Matthias Dellweg <2500@gmx.de> > > Date: Fri, 23 Sep 2011 19:32:39 +0200 > > Subject: [PATCH] drivers/usb/core/devio.c: Check for printer class > > specific request > > > > Signed-off-by: Matthias Dellweg <2500@gmx.de> > > --- > > drivers/usb/core/devio.c | 20 +++++++++++++++++--- > > 1 files changed, 17 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c > > index 37518df..e4175cc 100644 > > --- a/drivers/usb/core/devio.c > > +++ b/drivers/usb/core/devio.c > > @@ -607,9 +607,10 @@ static int findintfep(struct usb_device *dev, > > unsigned int ep) } > > > > static int check_ctrlrecip(struct dev_state *ps, unsigned int > > requesttype, > > - unsigned int index) > > + unsigned int request, unsigned int > > index) { > > int ret = 0; > > + struct usb_host_interface *alt_setting; > > > > if (ps->dev->state != USB_STATE_UNAUTHENTICATED > > && ps->dev->state != USB_STATE_ADDRESS > > @@ -618,6 +619,18 @@ static int check_ctrlrecip(struct dev_state > > *ps, unsigned int requesttype, if (USB_TYPE_VENDOR == > > (USB_TYPE_MASK & requesttype)) return 0; > > > > + /* > > + * check for the special corner case 'get_device_id' in > > the printer > > + * class specification, where wIndex is (interface << 8 | > > altsetting) > > + */ > > + if (requesttype == 0xa1 && request == 0) { > > + alt_setting = > > usb_find_alt_setting(ps->dev->actconfig, > > + index >> 8, > > index & 0xff); > > + if (alt_setting > > + && alt_setting->desc.bInterfaceClass == > > USB_CLASS_PRINTER) > > + index >>= 8; > > + } > > + > > index &= 0xff; > > switch (requesttype & USB_RECIP_MASK) { > > case USB_RECIP_ENDPOINT: > > @@ -770,7 +783,8 @@ static int proc_control(struct dev_state *ps, > > void __user *arg) > > if (copy_from_user(&ctrl, arg, sizeof(ctrl))) > > return -EFAULT; > > - ret = check_ctrlrecip(ps, ctrl.bRequestType, ctrl.wIndex); > > + ret = check_ctrlrecip(ps, ctrl.bRequestType, ctrl.bRequest, > > + ctrl.wIndex); > > if (ret) > > return ret; > > wLength = ctrl.wLength; /* To suppress 64k > > PAGE_SIZE warning */ @@ -1100,7 +1114,7 @@ static int > > proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, > > kfree(dr); return -EINVAL; > > } > > - ret = check_ctrlrecip(ps, dr->bRequestType, > > + ret = check_ctrlrecip(ps, dr->bRequestType, > > dr->bRequest, le16_to_cpup(&dr->wIndex)); > > if (ret) { > > kfree(dr); > > > > This looks fine to me. Greg may ask you for a non-empty patch > description when you submit it to him. > > Acked-by: Alan Stern <stern@rowland.harvard.edu> > > Alan Stern Hi Greg, I hear you are the right person to ask for including this patch. The relevant section in the printer class spec is 4.2.1. I believe this is a really special corner case, because I did not find another such 'violation' of the common usb spec on a quick search. The hardware I tested this patch with is a Prolific USB->Centronics cable (067b:2305). Matthias Dellweg >From 5bdb64cbee0b2f353d3bc444c147ba78f34a1f69 Mon Sep 17 00:00:00 2001 From: Matthias Dellweg <2500@gmx.de> Date: Sun, 25 Sep 2011 14:26:25 +0200 Subject: [PATCH] usb/core/devio.c: Check for printer class specific request In the usb printer class specific request get_device_id the value of wIndex is (interface << 8 | altsetting) instead of just interface. This enables the detection of some printers with libusb. Acked-by: Alan Stern <stern@rowland.harvard.edu> Signed-off-by: Matthias Dellweg <2500@gmx.de> --- drivers/usb/core/devio.c | 21 ++++++++++++++++++--- 1 files changed, 18 insertions(+), 3 deletions(-) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 37518df..1d73709 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -607,9 +607,10 @@ static int findintfep(struct usb_device *dev, unsigned int ep) } static int check_ctrlrecip(struct dev_state *ps, unsigned int requesttype, - unsigned int index) + unsigned int request, unsigned int index) { int ret = 0; + struct usb_host_interface *alt_setting; if (ps->dev->state != USB_STATE_UNAUTHENTICATED && ps->dev->state != USB_STATE_ADDRESS @@ -618,6 +619,19 @@ static int check_ctrlrecip(struct dev_state *ps, unsigned int requesttype, if (USB_TYPE_VENDOR == (USB_TYPE_MASK & requesttype)) return 0; + /* + * check for the special corner case 'get_device_id' in the printer + * class specification, where wIndex is (interface << 8 | altsetting) + * instead of just interface + */ + if (requesttype == 0xa1 && request == 0) { + alt_setting = usb_find_alt_setting(ps->dev->actconfig, + index >> 8, index & 0xff); + if (alt_setting + && alt_setting->desc.bInterfaceClass == USB_CLASS_PRINTER) + index >>= 8; + } + index &= 0xff; switch (requesttype & USB_RECIP_MASK) { case USB_RECIP_ENDPOINT: @@ -770,7 +784,8 @@ static int proc_control(struct dev_state *ps, void __user *arg) if (copy_from_user(&ctrl, arg, sizeof(ctrl))) return -EFAULT; - ret = check_ctrlrecip(ps, ctrl.bRequestType, ctrl.wIndex); + ret = check_ctrlrecip(ps, ctrl.bRequestType, ctrl.bRequest, + ctrl.wIndex); if (ret) return ret; wLength = ctrl.wLength; /* To suppress 64k PAGE_SIZE warning */ @@ -1100,7 +1115,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, kfree(dr); return -EINVAL; } - ret = check_ctrlrecip(ps, dr->bRequestType, + ret = check_ctrlrecip(ps, dr->bRequestType, dr->bRequest, le16_to_cpup(&dr->wIndex)); if (ret) { kfree(dr); -- 1.7.6.3 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] usb/core/devio.c: Check for printer class specific request 2011-09-25 12:46 ` Matthias Dellweg @ 2011-09-26 22:34 ` Greg KH 2011-09-26 23:24 ` Matthias Dellweg 0 siblings, 1 reply; 11+ messages in thread From: Greg KH @ 2011-09-26 22:34 UTC (permalink / raw) To: Matthias Dellweg; +Cc: Greg Kroah-Hartman, Alan Stern, linux-usb, linux-kernel On Sun, Sep 25, 2011 at 02:46:51PM +0200, Matthias Dellweg wrote: > From: Matthias Dellweg <2500@gmx.de> > > In the usb printer class specific request get_device_id the value of > wIndex is (interface << 8 | altsetting) instead of just interface. > This enables the detection of some printers with libusb. > > The hardware I tested this patch with is a Prolific > USB->Centronics cable (067b:2305). > > Acked-by: Alan Stern <stern@rowland.harvard.edu> > Signed-off-by: Matthias Dellweg <2500@gmx.de> > Cc: stable <stable@kernel.org> > --- > drivers/usb/core/devio.c | 21 ++++++++++++++++++--- > 1 files changed, 18 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c > index 37518df..1d73709 100644 > --- a/drivers/usb/core/devio.c > +++ b/drivers/usb/core/devio.c > @@ -607,9 +607,10 @@ static int findintfep(struct usb_device *dev, unsigned int ep) > } > > static int check_ctrlrecip(struct dev_state *ps, unsigned int requesttype, > - unsigned int index) > + unsigned int request, unsigned int index) > { > int ret = 0; > + struct usb_host_interface *alt_setting; > > if (ps->dev->state != USB_STATE_UNAUTHENTICATED > && ps->dev->state != USB_STATE_ADDRESS All of the tabs were stripped out of this email, making it impossible to apply it :( Care to fix this up and resend it so that I can apply it? thanks, greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] usb/core/devio.c: Check for printer class specific request 2011-09-26 22:34 ` [PATCH] usb/core/devio.c: Check for printer " Greg KH @ 2011-09-26 23:24 ` Matthias Dellweg 2011-09-26 23:31 ` Greg KH 0 siblings, 1 reply; 11+ messages in thread From: Matthias Dellweg @ 2011-09-26 23:24 UTC (permalink / raw) To: Greg KH; +Cc: Alan Stern, linux-usb, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1703 bytes --] Am Mon, 26 Sep 2011 15:34:30 -0700 schrieb Greg KH <greg@kroah.com>: > On Sun, Sep 25, 2011 at 02:46:51PM +0200, Matthias Dellweg wrote: > > From: Matthias Dellweg <2500@gmx.de> > > > > In the usb printer class specific request get_device_id the value of > > wIndex is (interface << 8 | altsetting) instead of just interface. > > This enables the detection of some printers with libusb. > > > > The hardware I tested this patch with is a Prolific > > USB->Centronics cable (067b:2305). > > > > Acked-by: Alan Stern <stern@rowland.harvard.edu> > > Signed-off-by: Matthias Dellweg <2500@gmx.de> > > Cc: stable <stable@kernel.org> > > --- > > drivers/usb/core/devio.c | 21 ++++++++++++++++++--- > > 1 files changed, 18 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c > > index 37518df..1d73709 100644 > > --- a/drivers/usb/core/devio.c > > +++ b/drivers/usb/core/devio.c > > @@ -607,9 +607,10 @@ static int findintfep(struct usb_device *dev, > > unsigned int ep) } > > > > static int check_ctrlrecip(struct dev_state *ps, unsigned int > > requesttype, > > - unsigned int index) > > + unsigned int request, unsigned int index) > > { > > int ret = 0; > > + struct usb_host_interface *alt_setting; > > > > if (ps->dev->state != USB_STATE_UNAUTHENTICATED > > && ps->dev->state != USB_STATE_ADDRESS > > All of the tabs were stripped out of this email, making it impossible > to apply it :( > > Care to fix this up and resend it so that I can apply it? > > thanks, > > greg k-h Again? I mean I'm sorry. Do you accept it as an attachment? Matthias Dellweg [-- Attachment #2: 0001-usb-core-devio.c-Check-for-printer-class-specific-re.patch --] [-- Type: text/x-patch, Size: 2580 bytes --] >From 5bdb64cbee0b2f353d3bc444c147ba78f34a1f69 Mon Sep 17 00:00:00 2001 From: Matthias Dellweg <2500@gmx.de> Date: Sun, 25 Sep 2011 14:26:25 +0200 Subject: [PATCH] usb/core/devio.c: Check for printer class specific request In the usb printer class specific request get_device_id the value of wIndex is (interface << 8 | altsetting) instead of just interface. This enables the detection of some printers with libusb. Acked-by: Alan Stern <stern@rowland.harvard.edu> Signed-off-by: Matthias Dellweg <2500@gmx.de> --- drivers/usb/core/devio.c | 21 ++++++++++++++++++--- 1 files changed, 18 insertions(+), 3 deletions(-) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 37518df..1d73709 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -607,9 +607,10 @@ static int findintfep(struct usb_device *dev, unsigned int ep) } static int check_ctrlrecip(struct dev_state *ps, unsigned int requesttype, - unsigned int index) + unsigned int request, unsigned int index) { int ret = 0; + struct usb_host_interface *alt_setting; if (ps->dev->state != USB_STATE_UNAUTHENTICATED && ps->dev->state != USB_STATE_ADDRESS @@ -618,6 +619,19 @@ static int check_ctrlrecip(struct dev_state *ps, unsigned int requesttype, if (USB_TYPE_VENDOR == (USB_TYPE_MASK & requesttype)) return 0; + /* + * check for the special corner case 'get_device_id' in the printer + * class specification, where wIndex is (interface << 8 | altsetting) + * instead of just interface + */ + if (requesttype == 0xa1 && request == 0) { + alt_setting = usb_find_alt_setting(ps->dev->actconfig, + index >> 8, index & 0xff); + if (alt_setting + && alt_setting->desc.bInterfaceClass == USB_CLASS_PRINTER) + index >>= 8; + } + index &= 0xff; switch (requesttype & USB_RECIP_MASK) { case USB_RECIP_ENDPOINT: @@ -770,7 +784,8 @@ static int proc_control(struct dev_state *ps, void __user *arg) if (copy_from_user(&ctrl, arg, sizeof(ctrl))) return -EFAULT; - ret = check_ctrlrecip(ps, ctrl.bRequestType, ctrl.wIndex); + ret = check_ctrlrecip(ps, ctrl.bRequestType, ctrl.bRequest, + ctrl.wIndex); if (ret) return ret; wLength = ctrl.wLength; /* To suppress 64k PAGE_SIZE warning */ @@ -1100,7 +1115,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, kfree(dr); return -EINVAL; } - ret = check_ctrlrecip(ps, dr->bRequestType, + ret = check_ctrlrecip(ps, dr->bRequestType, dr->bRequest, le16_to_cpup(&dr->wIndex)); if (ret) { kfree(dr); -- 1.7.6.3 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] usb/core/devio.c: Check for printer class specific request 2011-09-26 23:24 ` Matthias Dellweg @ 2011-09-26 23:31 ` Greg KH 0 siblings, 0 replies; 11+ messages in thread From: Greg KH @ 2011-09-26 23:31 UTC (permalink / raw) To: Matthias Dellweg; +Cc: Alan Stern, linux-usb, linux-kernel On Tue, Sep 27, 2011 at 01:24:07AM +0200, Matthias Dellweg wrote: > Do you accept it as an attachment? That works, thanks. greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2011-09-26 23:33 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-09-22 10:10 [PATCH] enable usb control message with class specific request Matthias Dellweg 2011-09-22 15:12 ` Alan Stern 2011-09-22 21:56 ` Matthias Dellweg 2011-09-23 7:05 ` Matthias Dellweg 2011-09-23 16:16 ` Alan Stern 2011-09-23 17:57 ` Matthias Dellweg 2011-09-23 18:31 ` Alan Stern 2011-09-25 12:46 ` Matthias Dellweg 2011-09-26 22:34 ` [PATCH] usb/core/devio.c: Check for printer " Greg KH 2011-09-26 23:24 ` Matthias Dellweg 2011-09-26 23:31 ` Greg KH
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.