On Tue, Jan 31, 2017 at 05:55:17PM +0100, Greg Kroah-Hartman wrote:
> On Tue, Jan 31, 2017 at 05:46:02PM +0100, Johan Hovold wrote:
> > On Tue, Jan 31, 2017 at 05:41:52PM +0100, Greg Kroah-Hartman wrote:
> > > On Tue, Jan 31, 2017 at 05:17:28PM +0100, Johan Hovold wrote:
> > > > Make sure the received data has the required headers before parsing it.
> > > >
> > > > Also drop the redundant urb-status check, which has already been handled
> > > > by the caller.
> > > >
> > > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > > > Signed-off-by: Johan Hovold <[email protected]>
> > > > ---
> > > > drivers/usb/serial/digi_acceleport.c | 38
> > > > ++++++++++++++++++++++--------------
> > > > 1 file changed, 23 insertions(+), 15 deletions(-)
> > > >
> > > > diff --git a/drivers/usb/serial/digi_acceleport.c
> > > > b/drivers/usb/serial/digi_acceleport.c
> > > > index 3b610f1e3f7c..eb433922598c 100644
> > > > --- a/drivers/usb/serial/digi_acceleport.c
> > > > +++ b/drivers/usb/serial/digi_acceleport.c
> > > > @@ -1398,25 +1398,30 @@ static int digi_read_inb_callback(struct urb
> > > > *urb)
> > > > {
> > > > struct usb_serial_port *port = urb->context;
> > > > struct digi_port *priv = usb_get_serial_port_data(port);
> > > > - int opcode = ((unsigned char *)urb->transfer_buffer)[0];
> > > > - int len = ((unsigned char *)urb->transfer_buffer)[1];
> > > > - int port_status = ((unsigned char *)urb->transfer_buffer)[2];
> > > > - unsigned char *data = ((unsigned char *)urb->transfer_buffer) +
> > > > 3;
> > > > + unsigned char *buf = urb->transfer_buffer;
> > > > + int opcode;
> > > > + int len;
> > > > + int port_status;
> > > > + unsigned char *data;
> > > > int flag, throttled;
> > > > - int status = urb->status;
> > > > -
> > > > - /* do not process callbacks on closed ports */
> > > > - /* but do continue the read chain */
> > > > - if (urb->status == -ENOENT)
> > > > - return 0;
> > > >
> > > > /* short/multiple packet check */
> > > > + if (urb->actual_length < 2) {
> > > > + dev_warn(&port->dev, "short packet received\n");
> > > > + return -1;
> > >
> > > Again, real error number? -EINVAL? -EIO?
> > >
> > > > + }
> > > > +
> > > > + opcode = buf[0];
> > > > + len = buf[1];
> > > > +
> > > > if (urb->actual_length != len + 2) {
> > > > - dev_err(&port->dev, "%s: INCOMPLETE OR MULTIPLE PACKET,
> > > > "
> > > > - "status=%d, port=%d, opcode=%d, len=%d, "
> > > > - "actual_length=%d, status=%d\n", __func__,
> > > > status,
> > > > - priv->dp_port_num, opcode, len,
> > > > urb->actual_length,
> > > > - port_status);
> > > > + dev_err(&port->dev, "malformed packet received:
> > > > port=%d, opcode=%d, len=%d, actual_length=%u\n",
> > > > + priv->dp_port_num, opcode, len,
> > > > urb->actual_length);
> > > > + return -1;
> > >
> > > Same here and elsewhere in this patch.
> >
> > As the OOB function in the previous patch, this one is also documented
> > as returning -1 on sanity-check failures so I'm not changing that
> > behaviour now.
> >
> > Also note that the return value is only checked against zero and never
> > used for anything else currently.
>
> Ok, nevermind, my fault.
>
> Reviewed-by: Greg Kroah-Hartman <[email protected]>
Thanks for reviewing these. Applying for -next.
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
