[U-Boot] [PATCH] usb: kbd: Fix dangling pointers on probe fail

[Bin Meng]

Hi Rob,

On Wed, Oct 4, 2017 at 1:31 AM, Rob Clark <robdclark at gmail.com> wrote:
> If probe fails, we should unregister the stdio device, else we leave
> dangling pointers to the parent 'struct usb_device'.
>
> Signed-off-by: Rob Clark <robdclark at gmail.com>
> ---
> I finally got around to debugging why things explode so badly without
> fixing usb_kbd vs. iomux[1] (which this patch applies on top of).
>
> [1] https://patchwork.ozlabs.org/patch/818881/
>
>  common/usb_kbd.c | 30 ++++++++++++++++++++++++------
>  1 file changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/common/usb_kbd.c b/common/usb_kbd.c
> index 4c3ad95fca..82ad93f6ca 100644
> --- a/common/usb_kbd.c
> +++ b/common/usb_kbd.c
> @@ -535,22 +535,40 @@ static int probe_usb_keyboard(struct usb_device *dev)
>                 error = iomux_doenv(stdin, stdinname);
>                 free(newstdin);
>                 if (error)
> -                       return error;
> +                       goto unregister_stdio;
>         } else {
>                 /* Check if this is the standard input device. */
> -               if (strcmp(stdinname, DEVNAME))
> -                       return 1;
> +               if (strcmp(stdinname, DEVNAME)) {
> +                       error = -1;

Could we use some meaningful errno?

> +                       goto unregister_stdio;
> +               }
>
>                 /* Reassign the console */
> -               if (overwrite_console())
> -                       return 1;
> +               if (overwrite_console()) {
> +                       error = -1;
> +                       goto unregister_stdio;
> +               }
>
>                 error = console_assign(stdin, DEVNAME);
>                 if (error)
> -                       return error;
> +                       goto unregister_stdio;
>         }
>
>         return 0;
> +
> +unregister_stdio:
> +       /*
> +        * If probe fails, the device will be removed.. leaving dangling
> +        * pointers if the stdio device is not unregistered.  If u-boot

nits: U-Boot

> +        * is built without stdio_deregister(), just pretend to succeed
> +        * in order to avoid dangling pointers.
> +        */
> +#if CONFIG_IS_ENABLED(SYS_STDIO_DEREGISTER)
> +       stdio_deregister(DEVNAME, 1);
> +       return error;
> +#else
> +       return 0;

Don't understand why 'return 0' here if probe fails...

> +#endif
>  }
>
>  #ifndef CONFIG_DM_USB
> --

Regards,
Bin