USB: serial: console: fix two use-after-free bugs

PATCH In review Johan Hovold 4 participants 9 messages Oct 4, 2017 – Oct 9, 2017

Subsystems (3)

Stable branch USB USB serial

lore

Share & tools

lore

Johan Hovold

author maintainer

2017-10-04 09:01 UTC #1 lore raw

PATCH 1/2 USB: serial: console: fix use-after-free on disconnect

In reply to Johan Hovold

A clean-up patch removing removing two redundant NULL-checks from the console disconnect handler inadvertently also removed a third check. This could lead to the struct usb_serial being prematurely freed by the console code when a driver accepts but does not register any ports for an interface which also lacks endpoint descriptors.

Fixes 0e517c93dc02 ("USB: serial: console: clean up sanity checks")

Cc stable <[email protected]> # 4.11

Reported-by Andrey Konovalov <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/console.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/usb/serial/console.c +1 -1

index fdf89800ebc3..ed8ba3ef5c79 100644
@@ -265,7 +265,7 @@ static struct console usbcons = {
 
 void usb_serial_console_disconnect(struct usb_serial *serial)
 {
-	if (serial->port[0] == usbcons_info.port) {
+	if (serial->port[0] && serial->port[0] == usbcons_info.port) {
 		usb_serial_console_exit();
 		usb_serial_put(serial);
 	}

Signature · 1 line

2.14.2

Johan Hovold

author maintainer

2017-10-04 09:01 UTC #7 lore raw

PATCH 2/2 USB: serial: console: fix use-after-free after failed setup

In reply to Johan Hovold

Make sure to reset the USB-console port pointer when console setup fails in order to avoid having the struct usb_serial be prematurely freed by the console code when the device is later disconnected.

Fixes 73e487fdb75f ("[PATCH] USB console: fix disconnection issues")

Cc stable <[email protected]> # 2.6.18

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/console.c | 1 +
 1 file changed, 1 insertion(+)

drivers/usb/serial/console.c +1 -0

index ed8ba3ef5c79..43a862a90a77 100644
@@ -186,6 +186,7 @@ static int usb_console_setup(struct console *co, char *options)
 	tty_kref_put(tty);
  reset_open_count:
 	port->port.count = 0;
+	info->port = NULL;
 	usb_autopm_put_interface(serial->interface);
  error_get_interface:
 	usb_serial_put(serial);

Signature · 1 line

2.14.2

Johan Hovold

author maintainer

2017-10-04 09:02 UTC #0 lore raw

cover USB: serial: console: fix two use-after-free bugs

A recent clean-up patch of mine did more than intended and introduced the potential for a use-after-free on disconnect under some very specific circumstances. Fortunately those circumstances were not obscure enough to prevent Andrey's fuzzing from triggering the bug.

While fixing this one I found a related issue through inspection which dates back to 2006.

Johan

Johan Hovold (2):
USB: serial: console: fix use-after-free on disconnect
USB: serial: console: fix use-after-free after failed setup

 drivers/usb/serial/console.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Signature · 1 line

2.14.2

Greg KH

Acked-by Greg Kroah-Hartman on v1 2/2

2017-10-04 14:39 UTC #8 lore raw

Re: [PATCH 2/2] USB: serial: console: fix use-after-free after failed setup

In reply to Johan Hovold

Quoted 7 lines · Johan Hovold

Make sure to reset the USB-console port pointer when console setup fails in order to avoid having the struct usb_serial be prematurely freed by the console code when the device is later disconnected.
Fixes 73e487fdb75f ("[PATCH] USB console: fix disconnection issues")
Cc stable <[email protected]> # 2.6.18
Signed-off-by Johan Hovold <[email protected]>

Acked-by Greg Kroah-Hartman <[email protected]>

Greg KH

Acked-by Greg Kroah-Hartman on v1 1/2

2017-10-04 14:39 UTC #2 lore raw

Re: [PATCH 1/2] USB: serial: console: fix use-after-free on disconnect

In reply to Johan Hovold

Quoted 10 lines · Johan Hovold

A clean-up patch removing removing two redundant NULL-checks from the console disconnect handler inadvertently also removed a third check. This could lead to the struct usb_serial being prematurely freed by the console code when a driver accepts but does not register any ports for an interface which also lacks endpoint descriptors.
Fixes 0e517c93dc02 ("USB: serial: console: clean up sanity checks")
Cc stable <[email protected]> # 4.11
Reported-by Andrey Konovalov <[email protected]>
Signed-off-by Johan Hovold <[email protected]>

Acked-by Greg Kroah-Hartman <[email protected]>

Jaejoong Kim

2017-10-07 08:56 UTC #3 lore raw

Re: [PATCH 1/2] USB: serial: console: fix use-after-free on disconnect

In reply to Johan Hovold

2017-10-04 18:01 GMT+09:00 Johan Hovold <[email protected]>:

Quoted 1 line

A clean-up patch removing removing two redundant NULL-checks from the

                               ^^^^^^^^^^^^^^^^^^

The word 'removing' was written twice. :)

Quoted 33 lines

console disconnect handler inadvertently also removed a third check. This could lead to the struct usb_serial being prematurely freed by the console code when a driver accepts but does not register any ports for an interface which also lacks endpoint descriptors.
Fixes 0e517c93dc02 ("USB: serial: console: clean up sanity checks")
Cc stable <[email protected]> # 4.11
Reported-by Andrey Konovalov <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/serial/console.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/serial/console.c +1 -2
index fdf89800ebc3..ed8ba3ef5c79 100644
@@ -265,7 +265,7 @@ static struct console usbcons = {
 
 void usb_serial_console_disconnect(struct usb_serial *serial)
 {
-       if (serial->port[0] == usbcons_info.port) {
+       if (serial->port[0] && serial->port[0] == usbcons_info.port) {
                usb_serial_console_exit();
                usb_serial_put(serial);
        }
--
2.14.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Johan Hovold

author maintainer

2017-10-09 10:41 UTC #4 lore raw

Re: [PATCH 1/2] USB: serial: console: fix use-after-free on disconnect

Quoted 6 lines · Jaejoong Kim

Hi
2017-10-04 18:01 GMT+09:00 Johan Hovold <[email protected]>:
Quoted 1 line
A clean-up patch removing removing two redundant NULL-checks from the
                               ^^^^^^^^^^^^^^^^^^
The word 'removing' was written twice. :)

Thanks for noticing, now fixed up.

Johan

Andrey Konovalov

Tested-by on v1 1/2

2017-10-09 11:05 UTC #5 lore raw

Re: [PATCH 1/2] USB: serial: console: fix use-after-free on disconnect

In reply to Johan Hovold

Quoted 9 lines · Johan Hovold

A clean-up patch removing removing two redundant NULL-checks from the console disconnect handler inadvertently also removed a third check. This could lead to the struct usb_serial being prematurely freed by the console code when a driver accepts but does not register any ports for an interface which also lacks endpoint descriptors.
Fixes 0e517c93dc02 ("USB: serial: console: clean up sanity checks")
Cc stable <[email protected]> # 4.11
Reported-by Andrey Konovalov <[email protected]>

Tested-by Andrey Konovalov <[email protected]>

This fixes the crash.

Thanks!

Quoted 21 lines

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/console.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/usb/serial/console.c +1 -2

index fdf89800ebc3..ed8ba3ef5c79 100644
@@ -265,7 +265,7 @@ static struct console usbcons = {
 
 void usb_serial_console_disconnect(struct usb_serial *serial)
 {
-       if (serial->port[0] == usbcons_info.port) {
+       if (serial->port[0] && serial->port[0] == usbcons_info.port) {
                usb_serial_console_exit();
                usb_serial_put(serial);
        }
--

2.14.2

Johan Hovold

author maintainer

2017-10-09 11:10 UTC #6 lore raw

Re: [PATCH 1/2] USB: serial: console: fix use-after-free on disconnect

Quoted 7 lines · Andrey Konovalov

Quoted 6 lines · Johan Hovold
A clean-up patch removing removing two redundant NULL-checks from the console disconnect handler inadvertently also removed a third check. This could lead to the struct usb_serial being prematurely freed by the console code when a driver accepts but does not register any ports for an interface which also lacks endpoint descriptors.

Quoted 7 lines

Quoted 3 lines
Fixes 0e517c93dc02 ("USB: serial: console: clean up sanity checks")
Cc stable <[email protected]> # 4.11
Reported-by Andrey Konovalov <[email protected]>
Tested-by Andrey Konovalov <[email protected]>
This fixes the crash.

I just forwarded this one in a pull-request to Greg, but thanks for testing nonetheless.

Johan