| Message ID | 20180429154155.GA23102@kroah.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Apr 29, 2018 at 05:41:55PM +0200, Greg Kroah-Hartman wrote: > If we get an invalid device configuration from a palm 3 type device, we > might incorrectly parse things, and we have the potential to crash in > "interesting" ways. > > Fix this up by verifying the size of the configuration passed to us by > the device, and only if it is correct, will we handle it. > > Reported-by: Andrey Konovalov <andreyknvl@google.com> > Reviewed-by: Andrey Konovalov <andreyknvl@google.com> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > --- > > Here is my long-forgotten patch for the visor driver to resolve an issue > that Andrey found back in September of 2017. Sorry for the long delay. > > Johan, I incorporated your review comments of my original one-off patch > here as well. Thanks for the update. I've applied this for 4.17-rc now after adding a stable tag and a comment about this one also fixing a slab info leak (through that dev_info in the port loop below). > diff --git a/drivers/usb/serial/visor.c b/drivers/usb/serial/visor.c > index f5373ed2cd45..8ddbecc25d89 100644 > --- a/drivers/usb/serial/visor.c > +++ b/drivers/usb/serial/visor.c > @@ -335,47 +335,48 @@ static int palm_os_3_probe(struct usb_serial *serial, > goto exit; > } > > - if (retval == sizeof(*connection_info)) { > - connection_info = (struct visor_connection_info *) > - transfer_buffer; > - > - num_ports = le16_to_cpu(connection_info->num_ports); > - for (i = 0; i < num_ports; ++i) { > - switch ( > - connection_info->connections[i].port_function_id) { > - case VISOR_FUNCTION_GENERIC: > - string = "Generic"; > - break; > - case VISOR_FUNCTION_DEBUGGER: > - string = "Debugger"; > - break; > - case VISOR_FUNCTION_HOTSYNC: > - string = "HotSync"; > - break; > - case VISOR_FUNCTION_CONSOLE: > - string = "Console"; > - break; > - case VISOR_FUNCTION_REMOTE_FILE_SYS: > - string = "Remote File System"; > - break; > - default: > - string = "unknown"; > - break; > - } > - dev_info(dev, "%s: port %d, is for %s use\n", > - serial->type->description, > - connection_info->connections[i].port, string); > - } Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, May 02, 2018 at 09:50:04AM +0200, Johan Hovold wrote: > On Sun, Apr 29, 2018 at 05:41:55PM +0200, Greg Kroah-Hartman wrote: > > If we get an invalid device configuration from a palm 3 type device, we > > might incorrectly parse things, and we have the potential to crash in > > "interesting" ways. > > > > Fix this up by verifying the size of the configuration passed to us by > > the device, and only if it is correct, will we handle it. > > > > Reported-by: Andrey Konovalov <andreyknvl@google.com> > > Reviewed-by: Andrey Konovalov <andreyknvl@google.com> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > > --- > > > > Here is my long-forgotten patch for the visor driver to resolve an issue > > that Andrey found back in September of 2017. Sorry for the long delay. > > > > Johan, I incorporated your review comments of my original one-off patch > > here as well. > > Thanks for the update. I've applied this for 4.17-rc now after adding a > stable tag and a comment about this one also fixing a slab info leak > (through that dev_info in the port loop below). Ugh, I forgot about the stable tag, thanks for that! greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/usb/serial/visor.c b/drivers/usb/serial/visor.c index f5373ed2cd45..8ddbecc25d89 100644 --- a/drivers/usb/serial/visor.c +++ b/drivers/usb/serial/visor.c @@ -335,47 +335,48 @@ static int palm_os_3_probe(struct usb_serial *serial, goto exit; } - if (retval == sizeof(*connection_info)) { - connection_info = (struct visor_connection_info *) - transfer_buffer; - - num_ports = le16_to_cpu(connection_info->num_ports); - for (i = 0; i < num_ports; ++i) { - switch ( - connection_info->connections[i].port_function_id) { - case VISOR_FUNCTION_GENERIC: - string = "Generic"; - break; - case VISOR_FUNCTION_DEBUGGER: - string = "Debugger"; - break; - case VISOR_FUNCTION_HOTSYNC: - string = "HotSync"; - break; - case VISOR_FUNCTION_CONSOLE: - string = "Console"; - break; - case VISOR_FUNCTION_REMOTE_FILE_SYS: - string = "Remote File System"; - break; - default: - string = "unknown"; - break; - } - dev_info(dev, "%s: port %d, is for %s use\n", - serial->type->description, - connection_info->connections[i].port, string); - } + if (retval != sizeof(*connection_info)) { + dev_err(dev, "Invalid connection information received from device\n"); + retval = -ENODEV; + goto exit; } - /* - * Handle devices that report invalid stuff here. - */ + + connection_info = (struct visor_connection_info *)transfer_buffer; + + num_ports = le16_to_cpu(connection_info->num_ports); + + /* Handle devices that report invalid stuff here. */ if (num_ports == 0 || num_ports > 2) { dev_warn(dev, "%s: No valid connect info available\n", serial->type->description); num_ports = 2; } + for (i = 0; i < num_ports; ++i) { + switch (connection_info->connections[i].port_function_id) { + case VISOR_FUNCTION_GENERIC: + string = "Generic"; + break; + case VISOR_FUNCTION_DEBUGGER: + string = "Debugger"; + break; + case VISOR_FUNCTION_HOTSYNC: + string = "HotSync"; + break; + case VISOR_FUNCTION_CONSOLE: + string = "Console"; + break; + case VISOR_FUNCTION_REMOTE_FILE_SYS: + string = "Remote File System"; + break; + default: + string = "unknown"; + break; + } + dev_info(dev, "%s: port %d, is for %s use\n", + serial->type->description, + connection_info->connections[i].port, string); + } dev_info(dev, "%s: Number of ports: %d\n", serial->type->description, num_ports);