Re: [PATCH] qla2xxx: Fix dpc_thread race on the module unload

[Gal Rosen <galr@storwize.com>]

From: James Bottomley <James.Bottomley@Ha...> - 2008-07-29 15:28

On Tue, 2008-07-29 at 19:13 +0400, Vladislav Bolkhovitin wrote:
> James Bottomley wrote:
> > On Tue, 2008-07-29 at 13:32 +0400, Vladislav Bolkhovitin wrote:
> >> Nope, taking only one that hunk from this patch isn't sufficient.
> >> Around 
> >> dpc_thread there is pretty simple and classical race. You can't do
> >>
> >> if (x != NULL)
> >> y = *x;
> >>
> >> without any protection, if x can be set to NULL by another thread.
It 
> >> can happen exactly between "if" and "*x" and hence lead to a crash,
> >> correct?
> > 
> > No.
> 
> What "No"? The above unlocked "if (x != NULL) y = *x;" is always safe 
> now? ;)

No ... no as in your analysis based on the example is not correct to
conclude protection is required. We have quite a number of examples of
this within the linux kernel (the SCSI error thread would be one).

But the wake up of the SCSI error thread is also called by holding a
spinlock (but not to protect the stopping of the thread).
The difference here is that the assignment of the thread to NULL is in
the thread function, when exiting from the while loop, and not before
calling kthread_stop() routine (like in the qla).
Maybe this would be the solution.