linux-kernel - Re: [PATCH] usb: core: Null deref in kernel with USB webcams.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <X61rce8GANHW1ysh@kroah.com>
Date:   Thu, 12 Nov 2020 18:05:53 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     John Boero <boeroboy@...il.com>
Cc:     Felipe Balbi <balbi@...nel.org>, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] usb: core: Null deref in kernel with USB webcams.

On Thu, Nov 12, 2020 at 03:52:02PM +0000, John Boero wrote:
> >From 54f9886454e9a28e8d943c1cef15df9c11555df7 Mon Sep 17 00:00:00 2001
> From: JohnnyB <jboero@...rs.noreply.github.com>

Why all this header here?

And the from: line doesn't match your Signed-off-by: line :(

> Date: Thu, 12 Nov 2020 15:28:29 +0000
> Subject: [PATCH] usb: core: Null deref in kernel with USB webcams.
> 
> Fixes: Ubuntu Launchpad bug 1827452
> 
> This is my first attempt at a kernel contribution so sorry if sloppy.

No need to put this in the changelog text and have it be in the kernel
for foever :)

> 
> There is some kind of race condition affecting Logitech
> webcams that crash USB with a null dereference.
> Affects raspberry pi devices as well as x86.
> No check on dev before dereference.
> Simple fix for issue experienced for months in
> both x86 and arm/rpi environments.
> 
> Signed-off-by: John Boero <boeroboy@...il.com>
> 
> ---
> drivers/usb/core/usb.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index d8756ffe513a..9b4ac4415f1a 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -272,13 +272,9 @@ EXPORT_SYMBOL_GPL(usb_find_alt_setting);
> struct usb_interface *usb_ifnum_to_if(const struct usb_device *dev,
>                                      unsigned ifnum)
> {
> -       struct usb_host_config *config = NULL;
> +       struct usb_host_config *config = dev->actconfig;
>        int i;
> 
> -       if (!dev)
> -               return NULL;
> -
> -       config = dev->actconfig;
>        if (!config)
>                return NULL;
>        for (i = 0; i < config->desc.bNumInterfaces; i++)

This patch is corrupted and can not be applied, but also, it looks
backwards, right?

And how about we find the race condition and fix that instead of trying
to paper over it here?

thanks,

greg k-h