usb: Fix null-ptr-deref in qt2_process_read_urb()
Subsystems (2)
PATCH v2 1/1 usb: Fix null-ptr-deref in qt2_process_read_urb()
This patch addresses a null-ptr-deref in qt2_process_read_urb() due to an incorrect bounds check in the following:
""
if (newport > serial->num_ports) {
dev_err(&port->dev,
"%s - port change to invalid port: %i\n",
__func__, newport);
break;
}""
The condition doesn't account for the valid range of the serial->port buffer, which is from 0 to serial->num_ports - 1. When newport is equal to serial->num_ports, the assignment of "port" in the following code is out-of-bounds and NULL:
""
serial_priv->current_port = newport;
port = serial->port[serial_priv->current_port];""
The fix checks if newport is greater than or equal to serial->num_ports indicating it is out-of-bounds.
--- drivers/usb/serial/quatech2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/serial/quatech2.c +1 -1
@@ -503,7 +503,7 @@ static void qt2_process_read_urb(struct urb *urb) newport = *(ch +3 ); -if (newport > serial->num_ports) { +if (newport >= serial->num_ports) {dev_err (&port->dev,"%s - port change to invalid port: %i\n" , __func__, newport);
Signature · 1 line
2.39.5
Quoted 21 lines · Qasim Ijaz
This patch addresses a null-ptr-deref in
qt2_process_read_urb()due to an incorrect bounds check in the following:""
if (newport > serial->num_ports) { dev_err(&port->dev, "%s - port change to invalid port: %i\n", __func__, newport); break; }""
The condition doesn't account for the valid range of the
serial->portbuffer, which is from 0 toserial->num_ports- 1. When newport is equal toserial->num_ports, the assignment of "port" in the following code is out-of-bounds and NULL:""
serial_priv->current_port = newport; port = serial->port[serial_priv->current_port];
Quoted 7 lines
""
The fix checks if newport is greater than or equal to
serial->num_portsindicating it is out-of-bounds.
Oh wow, syzbot is now starting to fuzz random USB devices? And this is the only thing it found for this device? That's not all that bad, I expected much worse.
Quoted 10 lines
Signed-off-by Qasim Ijaz <[email protected]>--- drivers/usb/serial/quatech2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/usb/serial/quatech2.c +0
-0
drivers/usb/serial/quatech2.c static void qt2_process_read_urb(struct urb *urb) · line 509
Quoted 8 lines
drivers/usb/serial/quatech2.c +1
-1@@ -503,7 +503,7 @@ static void qt2_process_read_urb(struct urb *urb) newport = *(ch +3 ); -if (newport > serial->num_ports) { +if (newport >= serial->num_ports) {dev_err (&port->dev,"%s - port change to invalid port: %i\n" , __func__, newport);
Johan Hovold
maintainerQuoted 2 lines · Qasim Ijaz
This patch addresses a null-ptr-deref in
qt2_process_read_urb()due to an incorrect bounds check in the following:
Thanks for the fix. This looks correct, but your patch is whitespace damaged (tabs replaced with spaces) and does not apply.
Can you please fix your mail setup and resend a v3? (Make sure to send it to yourself first.)
When doing so, please use the common patch prefix for this driver:
USB: serial: quatech2:
Quoted 1 line
""
You can drop these (markup?) quotes.
Quoted 17 lines
if (newport > serial->num_ports) { dev_err(&port->dev, "%s - port change to invalid port: %i\n", __func__, newport); break; }""
The condition doesn't account for the valid range of the
serial->portbuffer, which is from 0 toserial->num_ports- 1. When newport is equal toserial->num_ports, the assignment of "port" in the following code is out-of-bounds and NULL:""
serial_priv->current_port = newport; port = serial->port[serial_priv->current_port];
Quoted 8 lines
""
The fix checks if newport is greater than or equal to
serial->num_portsindicating it is out-of-bounds.Reported-by syzbot <[email protected]>Signed-off-by Qasim Ijaz <[email protected]>
And please include a Fixes and CC stable tag:
Fixes: f7a33e608d9a ("USB: serial: add quatech2 usb to serial driver")
Cc: [email protected] # 3.5Johan