diff mbox series

[v1,11/11] usb: kbd: destroy device after console is stopped

Message ID 20210211150944.73252-11-andriy.shevchenko@linux.intel.com
State Accepted
Commit eb5fd9e46c11ea41430d9c5bcc81d4583424216e
Delegated to: Tom Rini
Headers show
Series [v1,01/11] stdio: Get rid of dead code, i.e. stdio_deregister() | expand

Commit Message

Andy Shevchenko Feb. 11, 2021, 3:09 p.m. UTC
In case of IOMUX enabled it assumes that console devices in the list
are available to get them stopped properly via ->stop() callback.
However, the USB keyboard driver violates this assumption and tries
to play tricks so the device get destroyed while being listed as
an active console.

Swap the order of device deregistration and IOMUX update along with
converting to use iomux_replace_device() jelper to avoid the use-after-free.

Fixes: 3cbcb2892809 ("usb: Fix usb_kbd_deregister when console-muxing is used")
Fixes: 8a8348703081 ("dm: usb: Add a remove() method for USB keyboards")
Reported-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 common/usb_kbd.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Tom Rini Feb. 16, 2021, 9:57 p.m. UTC | #1
On Thu, Feb 11, 2021 at 05:09:44PM +0200, Andy Shevchenko wrote:

> In case of IOMUX enabled it assumes that console devices in the list
> are available to get them stopped properly via ->stop() callback.
> However, the USB keyboard driver violates this assumption and tries
> to play tricks so the device get destroyed while being listed as
> an active console.
> 
> Swap the order of device deregistration and IOMUX update along with
> converting to use iomux_replace_device() jelper to avoid the use-after-free.
> 
> Fixes: 3cbcb2892809 ("usb: Fix usb_kbd_deregister when console-muxing is used")
> Fixes: 8a8348703081 ("dm: usb: Add a remove() method for USB keyboards")
> Reported-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied to u-boot/master, thanks!
diff mbox series

Patch

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index b316807844b1..60c6027e048d 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -617,12 +617,12 @@  int usb_kbd_deregister(int force)
 	if (dev) {
 		usb_kbd_dev = (struct usb_device *)dev->priv;
 		data = usb_kbd_dev->privptr;
-		if (stdio_deregister_dev(dev, force) != 0)
-			return 1;
 #if CONFIG_IS_ENABLED(CONSOLE_MUX)
-		if (iomux_doenv(stdin, env_get("stdin")) != 0)
+		if (iomux_replace_device(stdin, DEVNAME, force ? "nulldev" : ""))
 			return 1;
 #endif
+		if (stdio_deregister_dev(dev, force) != 0)
+			return 1;
 #ifdef CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE
 		destroy_int_queue(usb_kbd_dev, data->intq);
 #endif
@@ -660,16 +660,16 @@  static int usb_kbd_remove(struct udevice *dev)
 		goto err;
 	}
 	data = udev->privptr;
-	if (stdio_deregister_dev(sdev, true)) {
-		ret = -EPERM;
-		goto err;
-	}
 #if CONFIG_IS_ENABLED(CONSOLE_MUX)
-	if (iomux_doenv(stdin, env_get("stdin"))) {
+	if (iomux_replace_device(stdin, DEVNAME, "nulldev")) {
 		ret = -ENOLINK;
 		goto err;
 	}
 #endif
+	if (stdio_deregister_dev(sdev, true)) {
+		ret = -EPERM;
+		goto err;
+	}
 #ifdef CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE
 	destroy_int_queue(udev, data->intq);
 #endif