Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1590406

Re: FW: RE: Re: Subject: [PATCH v3] USB:Core: BugFix: Proper handling of Race Condition when two USB class drivers try to call init_usb_class simultaneously

Show all headers | View raw

On Wed, 1 Mar 2017, Ajay Kaher wrote:

> > On Mon, 22 Feb 2017, Ajay Kaher wrote:
> > 
> >> On Mon, 20 Feb 2017, Ajay Kaher wrote:
> >> 
> >>> Alan, as per my understanding I have shifted the lock from
> >>> release_usb_class() to destroy_usb_class() in patch v3. 
> >>> If it is not right, please explain in detail which race condition
> >>> I have missed and also share your suggestions.
> >>> 
> >> 
> >> Have you considered what would happen if destroy_usb_class() ran, but 
> >> some other CPU was still holding a reference to usb_class?  And what if 
> >> the last reference gets dropped later on, while init_usb_class() is 
> >> running?
> > 
> > Access of usb_class->kref is only from either init_usb_class()
> > or destroy_usb_class(), and both these functions are now protected
> > with Mutex Locking in patch v3, so there is no chance of race condition
> > as per above scenarios.
> > 
> >> Maybe that's not possible here, but it is possible in general for 
> >> refcounted objects.  So yes, this code is probably okay, but it isn't 
> >> good form.
> > 
> > As per my understanding, I found to be one of the best possible solution
> > for this problem and this solutiuon don't have any side effect.
> 
> Alan, I had shared modified Patch v3 as per your inputs to prevent
> the race condition during simultaneously calling of init_usb_class().
> If you think there is scope to improve the patch, please share your inputs.

Under the circumstances, your patch is acceptable.

If you really want to make the point crystal clear, you could replace 
usb_class->kref with an ordinary integer counter.  Then it would be 
obvious that there are no references other than the ones taken by 
init_usb_class() and released by destroy_usb_class().

Alan Stern

> thanks,
> ajay kaher
> 
> 
> Signed-off-by: Ajay Kaher
> 
> ---
> 
>  drivers/usb/core/file.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/usb/core/file.c b/drivers/usb/core/file.c
> index 822ced9..a12d184 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,10 @@ static void release_usb_class(struct kref *kref)
> 
>  static void destroy_usb_class(void)
>  {
> +       mutex_lock(&init_usb_class_mutex);
>         if (usb_class)
>                 kref_put(&usb_class->kref, release_usb_class);
> +       mutex_unlock(&init_usb_class_mutex);
>  }
> 
>  int usb_major_init(void)
> @@ -171,7 +174,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;
> 
>  
> 

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

RE: Re: Subject: [PATCH v3] USB:Core: BugFix: Proper handling of Race  Condition when two USB class drivers try to call init_usb_class simultaneously Ajay Kaher <ajay.kaher@samsung.com> - 2017-02-22 14:30 +0100
  FW: RE: Re: Subject: [PATCH v3] USB:Core: BugFix: Proper handling of  Race Condition when two USB class drivers try to call init_usb_class  simultaneously Ajay Kaher <ajay.kaher@samsung.com> - 2017-03-01 13:40 +0100
    Re: FW: RE: Re: Subject: [PATCH v3] USB:Core: BugFix: Proper handling  of Race Condition when two USB class drivers try to call init_usb_class  simultaneously Alan Stern <stern@rowland.harvard.edu> - 2017-03-01 16:30 +0100
      FW: RE: Re: FW: RE: Re: Subject: [PATCH v3] USB:Core: BugFix: Proper  handling of Race Condition when two USB class drivers try to call  init_usb_class simultaneously Ajay Kaher <ajay.kaher@samsung.com> - 2017-03-02 09:40 +0100
        FW: FW: RE: Re: FW: RE: Re: Subject: [PATCH v3] USB:Core: BugFix:  Proper handling of Race Condition when two USB class drivers try to call  init_usb_class simultaneously Ajay Kaher <ajay.kaher@samsung.com> - 2017-03-03 16:30 +0100
          Re: FW: FW: RE: Re: FW: RE: Re: Subject: [PATCH v3] USB:Core: BugFix:  Proper handling of Race Condition when two USB class drivers try to call  init_usb_class simultaneously Alan Stern <stern@rowland.harvard.edu> - 2017-03-03 18:00 +0100
            Re: Subject: [PATCH v4] USB:Core: BugFix: Proper handling of Race  Condition when two USB class drivers try to call init_usb_class simultaneously Ajay Kaher <ajay.kaher@samsung.com> - 2017-03-07 07:10 +0100
              Re: Subject: [PATCH v4] USB:Core: BugFix: Proper handling of Race  Condition when two USB class drivers try to call init_usb_class simultaneously Alan Stern <stern@rowland.harvard.edu> - 2017-03-07 20:10 +0100
                Re: Subject: [PATCH v4] USB:Core: BugFix: Proper handling of Race  Condition when two USB class drivers try to call init_usb_class  simultaneously "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org> - 2017-03-09 13:30 +0100
                Re: Subject: [PATCH v4] USB:Core: BugFix: Proper handling of Race  Condition when two USB class drivers try to call init_usb_class simultaneously Ajay Kaher <ajay.kaher@samsung.com> - 2017-03-09 13:30 +0100
              Re: Subject: [PATCH v4] USB:Core: BugFix: Proper handling of Race  Condition when two USB class drivers try to call init_usb_class  simultaneously "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org> - 2017-03-08 19:10 +0100

csiph-web