On Fri, Jan 23, 2015 at 11:02:19PM +0800, Adam Lee wrote:
> On Fri, Jan 23, 2015 at 09:44:38AM +0100, Oliver Neukum wrote:
> > On Fri, 2015-01-23 at 14:54 +0800, Adam Lee wrote:
> > > In my scenario(pull that device then plug into another usb port while
> > > booting), invalid descriptor accessing happens just like Simon reported.
> > > Checking length and ignoring the invalid descriptors works.
> >
> > Hi,
> >
> > interesting. Have a look at what I sent Greg last week.
> >
> > Regards
> > Oliver
> >
> > From 14f823958cb3e99646419f743f3176d1059c3282 Mon Sep 17 00:00:00 2001
> > From: Oliver Neukum <oneukum@xxxxxxx>
> > Date: Tue, 13 Jan 2015 16:55:52 +0100
> > Subject: [PATCH 1/4] cdc-acm: add sanity checks
> >
> > Check the special CDC headers for a plausible minimum length.
> > Another big operating systems ignores such garbage.
> >
> > Signed-off-by: Oliver Neukum <oneukum@xxxxxxx>
> > ---
> >
> > ...
> >
> > case USB_CDC_ACM_TYPE:
> > + if (elength < 3)
> > + break;
> > ac_management_function = buffer[3];
> > break;
> > case USB_CDC_CALL_MANAGEMENT_TYPE:
> > + if (elength < 4)
> > + break;
> > call_management_function = buffer[3];
> > call_interface_num = buffer[4];
> > break;
> >
> > ...
> >
> > next_desc:
> > - buflen -= buffer[0];
> > - buffer += buffer[0];
> > + buflen -= elength;
> > + buffer += elength;
> > }
> >
> > if (!union_header) {
> > --
> > 1.8.4.5
>
> Hi, Oliver
>
> Shouldn't the length checks be "if (elength < 4)" and "if (elength <
> 5)"? Think the logic of "buflen -= elength" and "buffer += elength",
> elength must count itself(buffer[0]) in.
Also I think you should "goto next_desc" but not "break".
--
Adam Lee
http://adam8157.info
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
