* Possible data-race related bug in u132_hcd module. @ 2020-03-30 11:52 madhuparnabhowmik10 2020-03-30 12:02 ` Greg KH 2020-03-30 16:03 ` Alan Stern 0 siblings, 2 replies; 5+ messages in thread From: madhuparnabhowmik10 @ 2020-03-30 11:52 UTC (permalink / raw) To: gregkh, hariprasad.kelam, colin.king, tony.olech Cc: linux-usb, linux-kernel, andrianov Hi, This bug is found by Linux Driver Verification project (linuxtesting.org). The bug is related to the parallel execution of u132_probe() function and u132_hcd_exit() function in u132_hcd.c. In case the module is unloaded when the probe function is executing there can be data race as the mutex lock u132_module_lock is not used properly. i) Usage of mutex lock only when writing into the u132_exiting variable in u132_hcd_exit(). The lock is not used when this variable is read in u132_probe(). Moreover, this variable does not serve its purpose, as even if locking is used while the u132_exiting variable is read in probe(), the function may still miss that exit function is executing if it acquires the mutex before exit() function does. How to fix this? ii) Usage of mutex while adding entries in u132_static_list in probe function but not in exit function while unregistering. This should be easy to fix by holding the mutex in the exit function as well. There can be other synchronization problems related to the usage of u132_module_lock in this module, I have only spotted these so far. Please let me know if this bug report is helpful and I can send a patch fixing it. Thank you, Madhuparna ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Possible data-race related bug in u132_hcd module. 2020-03-30 11:52 Possible data-race related bug in u132_hcd module madhuparnabhowmik10 @ 2020-03-30 12:02 ` Greg KH 2020-03-30 12:25 ` Madhuparna Bhowmik 2020-03-30 16:03 ` Alan Stern 1 sibling, 1 reply; 5+ messages in thread From: Greg KH @ 2020-03-30 12:02 UTC (permalink / raw) To: madhuparnabhowmik10 Cc: hariprasad.kelam, colin.king, tony.olech, linux-usb, linux-kernel, andrianov On Mon, Mar 30, 2020 at 05:22:43PM +0530, madhuparnabhowmik10@gmail.com wrote: > Hi, > > This bug is found by Linux Driver Verification project (linuxtesting.org). > > The bug is related to the parallel execution of u132_probe() function and u132_hcd_exit() function in u132_hcd.c. In case the module is unloaded when the probe function is executing there can be data race as the mutex lock u132_module_lock is not used properly. Please note that module unloading, while a nice thing to have, is never something that happens automatically :) > i) Usage of mutex lock only when writing into the u132_exiting variable in u132_hcd_exit(). The lock is not used when this variable is read in u132_probe(). > > Moreover, this variable does not serve its purpose, as even if locking is used while the u132_exiting variable is read in probe(), the function may still miss that exit function is executing if it acquires the mutex before exit() function does. > > How to fix this? > > ii) Usage of mutex while adding entries in u132_static_list in probe function but not in exit function while unregistering. > This should be easy to fix by holding the mutex in the exit function as well. > > There can be other synchronization problems related to the usage of u132_module_lock in this module, I have only spotted these so far. > > Please let me know if this bug report is helpful and I can send a patch fixing it. Please just send a patch, no need to ever ask if you should, that's the best way to report and fix anything. thanks, greg k-h ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Possible data-race related bug in u132_hcd module. 2020-03-30 12:02 ` Greg KH @ 2020-03-30 12:25 ` Madhuparna Bhowmik 0 siblings, 0 replies; 5+ messages in thread From: Madhuparna Bhowmik @ 2020-03-30 12:25 UTC (permalink / raw) To: Greg KH Cc: madhuparnabhowmik10, hariprasad.kelam, colin.king, tony.olech, linux-usb, linux-kernel, andrianov On Mon, Mar 30, 2020 at 02:02:07PM +0200, Greg KH wrote: > On Mon, Mar 30, 2020 at 05:22:43PM +0530, madhuparnabhowmik10@gmail.com wrote: > > Hi, > > > > This bug is found by Linux Driver Verification project (linuxtesting.org). > > > > The bug is related to the parallel execution of u132_probe() function and u132_hcd_exit() function in u132_hcd.c. In case the module is unloaded when the probe function is executing there can be data race as the mutex lock u132_module_lock is not used properly. > > Please note that module unloading, while a nice thing to have, is never > something that happens automatically :) > > > i) Usage of mutex lock only when writing into the u132_exiting variable in u132_hcd_exit(). The lock is not used when this variable is read in u132_probe(). > > > > Moreover, this variable does not serve its purpose, as even if locking is used while the u132_exiting variable is read in probe(), the function may still miss that exit function is executing if it acquires the mutex before exit() function does. > > > > How to fix this? > > > > ii) Usage of mutex while adding entries in u132_static_list in probe function but not in exit function while unregistering. > > This should be easy to fix by holding the mutex in the exit function as well. > > > > There can be other synchronization problems related to the usage of u132_module_lock in this module, I have only spotted these so far. > > > > Please let me know if this bug report is helpful and I can send a patch fixing it. > > Please just send a patch, no need to ever ask if you should, that's the > best way to report and fix anything. > Sure, I will do it. Thank you, Madhuparna > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Possible data-race related bug in u132_hcd module. 2020-03-30 11:52 Possible data-race related bug in u132_hcd module madhuparnabhowmik10 2020-03-30 12:02 ` Greg KH @ 2020-03-30 16:03 ` Alan Stern 2020-03-30 17:20 ` Madhuparna Bhowmik 1 sibling, 1 reply; 5+ messages in thread From: Alan Stern @ 2020-03-30 16:03 UTC (permalink / raw) To: madhuparnabhowmik10 Cc: gregkh, hariprasad.kelam, colin.king, tony.olech, linux-usb, linux-kernel, andrianov On Mon, 30 Mar 2020 madhuparnabhowmik10@gmail.com wrote: > Hi, > > This bug is found by Linux Driver Verification project (linuxtesting.org). > > The bug is related to the parallel execution of u132_probe() function > and u132_hcd_exit() function in u132_hcd.c. In case the module is > unloaded when the probe function is executing there can be data race > as the mutex lock u132_module_lock is not used properly. Normally drivers do not have to worry about races between their probe and exit routines. The exit routine should unregister the driver from its bus subsystem, and unregistration is supposed to wait until all probe and remove functions have finished executing. > i) Usage of mutex lock only when writing into the u132_exiting > variable in u132_hcd_exit(). The lock is not used when this variable > is read in u132_probe(). I'm not familiar with u132_hcd, but the probe routine shouldn't need to use and "exiting" variable at all. > > Moreover, this variable does not serve its purpose, as even if > locking is used while the u132_exiting variable is read in probe(), > the function may still miss that exit function is executing if it > acquires the mutex before exit() function does. > > How to fix this? Are you certain there really is a problem? > ii) Usage of mutex while adding entries in u132_static_list in probe > function but not in exit function while unregistering. > This should be easy to fix by holding the mutex in the exit function as well. Why does the driver need a static list? > There can be other synchronization problems related to the usage of > u132_module_lock in this module, I have only spotted these so far. You should look at other drivers for comparison. They don't have to face this kind of problem. u132_hcd should be similar to them. Alan Stern ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Possible data-race related bug in u132_hcd module. 2020-03-30 16:03 ` Alan Stern @ 2020-03-30 17:20 ` Madhuparna Bhowmik 0 siblings, 0 replies; 5+ messages in thread From: Madhuparna Bhowmik @ 2020-03-30 17:20 UTC (permalink / raw) To: Alan Stern Cc: madhuparnabhowmik10, gregkh, hariprasad.kelam, colin.king, tony.olech, linux-usb, linux-kernel, andrianov On Mon, Mar 30, 2020 at 12:03:31PM -0400, Alan Stern wrote: > On Mon, 30 Mar 2020 madhuparnabhowmik10@gmail.com wrote: > > > Hi, > > > > This bug is found by Linux Driver Verification project (linuxtesting.org). > > > > The bug is related to the parallel execution of u132_probe() function > > and u132_hcd_exit() function in u132_hcd.c. In case the module is > > unloaded when the probe function is executing there can be data race > > as the mutex lock u132_module_lock is not used properly. > > Normally drivers do not have to worry about races between their probe > and exit routines. The exit routine should unregister the driver from > its bus subsystem, and unregistration is supposed to wait until all > probe and remove functions have finished executing. > > > i) Usage of mutex lock only when writing into the u132_exiting > > variable in u132_hcd_exit(). The lock is not used when this variable > > is read in u132_probe(). > > I'm not familiar with u132_hcd, but the probe routine shouldn't need to > use and "exiting" variable at all. > Even I am not sure why it should use this variable to check, that's why I thought of asking in the mailing list. If the maintainers agree that we can remove this variable I can send a patch doing it. This variable is not used for any other purpose in the module, so removing it shouldn't be a problem. > > > > Moreover, this variable does not serve its purpose, as even if > > locking is used while the u132_exiting variable is read in probe(), > > the function may still miss that exit function is executing if it > > acquires the mutex before exit() function does. > > > > How to fix this? > > Are you certain there really is a problem? > If the variable u132_exiting is really used for the purpose of checking if the module is exiting then there might be a problem. If at all it is assumed that exit never races with probe function and it is always called after probe finishes or something, then this variable is not even required. And suppose there is possibility of a race condition then only holding the mutex in exit but not probe does not make sense. > > ii) Usage of mutex while adding entries in u132_static_list in probe > > function but not in exit function while unregistering. > > This should be easy to fix by holding the mutex in the exit function as well. > > Why does the driver need a static list? > I do not know much about this module so I cannot answer this. From the point of synchronization, this list is initialized in init and then only used in probe and exit function. And lock is only held in probe, if at all it is assumed that exit cannot race with probe, there is no need to use the mutex. > > There can be other synchronization problems related to the usage of > > u132_module_lock in this module, I have only spotted these so far. > > You should look at other drivers for comparison. They don't have to > face this kind of problem. u132_hcd should be similar to them. > Yes, I checked out a few others and I could not see any usage of such variable to check if module is exiting. Thank you, Madhuparna > Alan Stern > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-03-30 17:21 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-30 11:52 Possible data-race related bug in u132_hcd module madhuparnabhowmik10 2020-03-30 12:02 ` Greg KH 2020-03-30 12:25 ` Madhuparna Bhowmik 2020-03-30 16:03 ` Alan Stern 2020-03-30 17:20 ` Madhuparna Bhowmik
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).