From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Saenz Julienne Date: Thu, 28 Jan 2021 18:19:56 +0100 Subject: [PATCH v1] usb: kbd: destroy device after console is stopped In-Reply-To: <20210128165525.39826-1-andriy.shevchenko@linux.intel.com> References: <20210128165525.39826-1-andriy.shevchenko@linux.intel.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Andy, On Thu, 2021-01-28 at 18:55 +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 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 > Signed-off-by: Andy Shevchenko > --- > v2: Nicolas, can you test this one instead of yours? Sadly this doesn't seem to work, and breaks a bunch of other tests in the process. You can try it yourself by running: './test/py/test.py --bd sandbox --build' Regards, Nicolas > ?common/usb_kbd.c | 12 ++++++------ > ?1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/common/usb_kbd.c b/common/usb_kbd.c > index b316807844b1..e09d9f7794c8 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) > ? 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"))) { > ? 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 -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 488 bytes Desc: This is a digitally signed message part URL: