USB: serial: quatech2: Fix null-ptr-deref in qt2_process_read_urb()
Subsystems (3)
PATCH v3 1/1 USB: serial: quatech2: 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
Johan Hovold
maintainerQuoted 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];The fix checks if newport is greater than or equal to
serial->num_portsindicating it is out-of-bounds.
Quoted 6 lines
Reported-by syzbot <[email protected]>Fixes f7a33e608d9a ("USB: serial: add quatech2 usb to serial driver")Cc <[email protected]> # 3.5Signed-off-by Qasim Ijaz <[email protected]>---
Thanks for the update. I've applied the patch now after adding Greg's Reviewed-by tag (for v2).
For your future contributions, try to remember to include any Reviewed-by or Tested-by tags when updating the patch unless the changes are non-trivial.
There should typically also be a short change log here under the --- line to indicate what changes from previous versions.
It is also encouraged to write the commit message in imperative mood (add, change, fix) and to avoid the phrase "this patch". There are some more details in
Documentation/process/submitting-patches.rst
Something to keep in mind for the future, but this patch already looks really good.
Johan