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]

OnÂTue,Â21ÂFebÂ2017,ÂAlanÂSternÂ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.

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;
Â
Â
Â
Â
Â
Â
Â