On Tue, Jan 31, 2017 at 05:17:27PM +0100, Johan Hovold wrote:
> Make sure to check for short transfers to avoid underflow in a loop
> condition when parsing the receive buffer.
>
> Also fix an off-by-one error in the incomplete sanity check which could
> lead to invalid data being parsed.
>
> Fixes: 8c209e6782ca ("USB: make actual_length in struct urb field u32")
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable <stable@xxxxxxxxxxxxxxx> # v2.6.30
> Signed-off-by: Johan Hovold <johan@xxxxxxxxxx>
> ---
> drivers/usb/serial/digi_acceleport.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/serial/digi_acceleport.c b/drivers/usb/serial/digi_acceleport.c
> index 6a1df9e824ca..3b610f1e3f7c 100644
> --- a/drivers/usb/serial/digi_acceleport.c
> +++ b/drivers/usb/serial/digi_acceleport.c
> @@ -1482,16 +1482,20 @@ static int digi_read_oob_callback(struct urb *urb)
> struct usb_serial *serial = port->serial;
> struct tty_struct *tty;
> struct digi_port *priv = usb_get_serial_port_data(port);
> + unsigned char *buf = urb->transfer_buffer;
> int opcode, line, status, val;
> int i;
> unsigned int rts;
>
> + if (urb->actual_length < 4)
> + return -1;
A real error number perhaps?
> +
> /* handle each oob command */
> - for (i = 0; i < urb->actual_length - 3;) {
> - opcode = ((unsigned char *)urb->transfer_buffer)[i++];
> - line = ((unsigned char *)urb->transfer_buffer)[i++];
> - status = ((unsigned char *)urb->transfer_buffer)[i++];
> - val = ((unsigned char *)urb->transfer_buffer)[i++];
> + for (i = 0; i < urb->actual_length - 4; i += 4) {
> + opcode = buf[i];
> + line = buf[i + 1];
> + status = buf[i + 2];
> + val = buf[i + 3];
Other than that nit, looks good, thanks for fixing my really old bugs :)
Reviewed-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
greg k-h
--
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
