From: Tristan Madani sierra_instat_callback() reads request fields and one byte past the usb_ctrlrequest header without first checking that urb->actual_length is large enough. A short USB interrupt transfer could cause an out-of-bounds read. Add two length checks matching the pattern already present in the option driver (option_instat_callback): one for the request header before accessing bRequestType, and one for the line-state data byte inside the serial-state notification handler. Use dev_err() since failing these checks causes the interrupt URB not to be resubmitted. Fixes: 033a3fb980b041 ("USB: Sierra Wireless driver update") Cc: stable@vger.kernel.org Signed-off-by: Tristan Madani --- Changes in v2 (Johan Hovold): - Split into two checks: one for the header before accessing bRequestType, and one for the line-state data byte inside the serial-state handler, matching the option driver pattern - Use dev_err() instead of dev_dbg() since early return prevents further interrupts drivers/usb/serial/sierra.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c index 6e443aacae07d..03ed76dfbb2e3 100644 --- a/drivers/usb/serial/sierra.c +++ b/drivers/usb/serial/sierra.c @@ -575,10 +575,23 @@ static void sierra_instat_callback(struct urb *urb) __func__); return; } + if (urb->actual_length < sizeof(*req_pkt)) { + dev_err(&port->dev, "%s: short transfer\n", + __func__); + return; + } if ((req_pkt->bRequestType == 0xA1) && (req_pkt->bRequest == 0x20)) { int old_dcd_state; - unsigned char signals = *((unsigned char *) + unsigned char signals; + + if (urb->actual_length < sizeof(*req_pkt) + 1) { + dev_err(&port->dev, + "%s: short interrupt transfer\n", + __func__); + return; + } + signals = *((unsigned char *) urb->transfer_buffer + sizeof(struct usb_ctrlrequest)); -- 2.47.3