linux-kernel - Re: Subject: [PATCH v4] USB:Core: BugFix: Proper handling of Race Condition when two USB class drivers try to call init_usb

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: <20170309121041.GB13054@kroah.com>
Date:   Thu, 9 Mar 2017 13:10:41 +0100
From:   "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
To:     Ajay Kaher <ajay.kaher@...sung.com>
Cc:     Alan Stern <stern@...land.harvard.edu>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        AMAN DEEP <aman.deep@...sung.com>,
        HEMANSHU SRIVASTAVA <hemanshu.s@...sung.com>
Subject: Re: Subject: [PATCH v4] USB:Core: BugFix: Proper handling of Race
 Condition when two USB class drivers try to call init_usb_class
 simultaneously

On Thu, Mar 09, 2017 at 11:34:25AM +0000, Ajay Kaher wrote:
> From febeb10887d5026a489658fd9e911656e76038ac Mon Sep 17 00:00:00 2001
> From: Ajay Kaher <ajay.kaher@...sung.com>
> Date: Thu, 9 Mar 2017 16:07:54 +0530
> Subject: [PATCH v4] USB:Core: BugFix: Proper handling of Race Condition when two
>  USB class drivers try to call init_usb_class simultaneously

Why is your subject line have the word "subject" in it?

Please fix your email client so you don't have the whole git commit
header in the body of the email like you do here.

Also, no need to say "Core:" or "BugFix:"

> 
> There is race condition when two USB class drivers try to call
> init_usb_class at the same time and leads to crash.
> code path: probe->usb_register_dev->init_usb_class
>  
> To solve this, mutex locking has been added in init_usb_class() and 
> destroy_usb_class().
> 
> As pointed by Alan, removed "if (usb_class)" test from destroy_usb_class()
> because usb_class can never be NULL there.
> 
> Signed-off-by: Ajay Kaher <ajay.kaher@...sung.com>
> Acked-by: Alan Stern <stern@...land.harvard.edu>
> ---
>  drivers/usb/core/file.c |    9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/core/file.c b/drivers/usb/core/file.c
> index 822ced9..422ce7b 100644
> --- a/drivers/usb/core/file.c
> +++ b/drivers/usb/core/file.c
> @@ -27,6 +27,7 @@
>  #define MAX_USB_MINORS 256
>  static const struct file_operations *usb_minors[MAX_USB_MINORS];
>  static DECLARE_RWSEM(minor_rwsem);
> +static DEFINE_MUTEX(init_usb_class_mutex);
> 
>  static int usb_open(struct inode *inode, struct file *file)
>  {
> @@ -109,8 +110,9 @@ static void release_usb_class(struct kref *kref)
> 
>  static void destroy_usb_class(void)
>  {
> - if (usb_class)
> -  kref_put(&usb_class->kref, release_usb_class);
> + mutex_lock(&init_usb_class_mutex);
> + kref_put(&usb_class->kref, release_usb_class);
> + mutex_unlock(&init_usb_class_mutex);
>  }
> 
>  int usb_major_init(void)
> @@ -171,7 +173,10 @@ int usb_register_dev(struct usb_interface *intf,
>   if (intf->minor >= 0)
>    return -EADDRINUSE;
> 
> + mutex_lock(&init_usb_class_mutex);
>   retval = init_usb_class();
> + mutex_unlock(&init_usb_class_mutex);
> +
>   if (retval)
>    return retval;
> 

All tabs were turned into spaces and this patch can not be applied :(

Please fix up and try again.  Send a patch to yourself first to see if
it works properly before sending it to us.

greg k-h