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 <johan@xxxxxxxxxx>
> > ---
> > 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.
Johan
--
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
