[PATCH v1] usb: kbd: destroy device after console is stopped

[Nicolas Saenz Julienne]

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 <nsaenzjulienne at suse.de>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko at linux.intel.com>
> ---
> 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: <https://lists.denx.de/pipermail/u-boot/attachments/20210128/829347a0/attachment.sig>