| 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
| ||
|
Message-ID: <20110922235656.1f821c1f@horus>
Date: Thu, 22 Sep 2011 23:56:56 +0200
From: Matthias Dellweg <2500@....de>
To: Alan Stern <stern@...land.harvard.edu>
Cc: Greg Kroah-Hartman <gregkh@...e.de>,
Vasiliy Kulikov <segooon@...il.com>,
Michal Sojka <sojkam1@....cvut.cz>,
Arnd Bergmann <arnd@...db.de>, <linux-usb@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] enable usb control message with class specific request
Am Thu, 22 Sep 2011 11:12:51 -0400 (EDT)
schrieb Alan Stern <stern@...land.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@....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@....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
--
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/
Powered by blists - more mailing lists