On Tue, Jan 03, 2017 at 05:27:07PM +0100, Greg Kroah-Hartman wrote:
> On Tue, Jan 03, 2017 at 04:39:40PM +0100, Johan Hovold wrote:
> > Fix NULL-pointer dereference when clearing halt at open should the device
> > lack a bulk-out endpoint.
> >
> > Unable to handle kernel NULL pointer dereference at virtual address 00000030
> > ...
> > PC is at cyberjack_open+0x40/0x9c [cyberjack]
> >
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Cc: stable <stable@xxxxxxxxxxxxxxx>
> > Signed-off-by: Johan Hovold <johan@xxxxxxxxxx>
> > ---
> > drivers/usb/serial/cyberjack.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/usb/serial/cyberjack.c b/drivers/usb/serial/cyberjack.c
> > index 5f17a3b9916d..80260b08398b 100644
> > --- a/drivers/usb/serial/cyberjack.c
> > +++ b/drivers/usb/serial/cyberjack.c
> > @@ -50,6 +50,7 @@
> > #define CYBERJACK_PRODUCT_ID 0x0100
> >
> > /* Function prototypes */
> > +static int cyberjack_attach(struct usb_serial *serial);
> > static int cyberjack_port_probe(struct usb_serial_port *port);
> > static int cyberjack_port_remove(struct usb_serial_port *port);
> > static int cyberjack_open(struct tty_struct *tty,
> > @@ -77,6 +78,7 @@ static struct usb_serial_driver cyberjack_device = {
> > .description = "Reiner SCT Cyberjack USB card reader",
> > .id_table = id_table,
> > .num_ports = 1,
> > + .attach = cyberjack_attach,
> > .port_probe = cyberjack_port_probe,
> > .port_remove = cyberjack_port_remove,
> > .open = cyberjack_open,
> > @@ -100,6 +102,14 @@ struct cyberjack_private {
> > short wrsent; /* Data already sent */
> > };
> >
> > +static int cyberjack_attach(struct usb_serial *serial)
> > +{
> > + if (serial->num_bulk_out < serial->num_ports)
> > + return -ENODEV;
> > +
> > + return 0;
> > +}
>
> You end up doing much the same thing for most of these drivers, is there
> any way to do it in the usb-serial core instead?
>
> I've been playing with an idea to have a USB driver specify the number
> and types of endpoints it requires and have the core just not even call
> the probe function if that doesn't match up. That should solve lots of
> these issues, can't you do much the same type of thing here instead of
> requiring a callback to do this?
I've been playing with that same idea, but I wanted minimal fixes that
could be backported to the stable trees for this first. I also kept the
checks as loose as possible to avoid any regressions.
Note that there seems to have been a general mechanism for this that was
removed in 2008 (see 07c3b1a10016 ("USB: remove broken usb-serial
num_endpoints check")), possibly because the checks were too tight.
But since there appear to be exploits out there for this class of
issues, we should probably consider reintroducing it in some form (e.g.
in USB core or USB serial core).
> Hm, but you want to match up the number of ports with the number of
> bulk endpoint pairs. That's tricky...
Yeah, and that's very much device dependent too, and layouts can change
after firmware is loaded, etc.
> Anyway, I guess this is ok, I just get worried when I see a bunch of the
> same changes in a bunch of different drivers.
Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html