Possible reentrancy issue in be_iopoll

Possible reentrancy issue in be_iopoll

[Shlomo Pongratz]

Hi Jayamohan.

I think that there is a reentrancy issue in 
"drivers/scsi/be2iscsi/be_main.c::be_iopoll".
The driver creates "NAPI" context per core which is fine, however the 
above routine declares the ret variable as static!
Thus there is only one instance of this variable, so if this routine is 
called from more than one thread of execution the result is unpredictable.

         static unsigned int ret;
         .....

         ret = beiscsi_process_cq(pbe_eq);
<----------------------------------------------------------------------------------Another 
thread can enter here and change "ret".
         if (ret < budget) {
                ....
         }
<----------------------------------------------------------------------------------Another 
thread can enter here and change "ret".
         return ret;

I can't submit a patch without checking it first, which is impossible 
since I don't have the HW, but the fix seems trivial, just take out the 
static.

Best regards,

S.P.

RE: Possible reentrancy issue in be_iopoll

[Kallickal, Jayamohan]

Shlomo,

 Thanks for the review and spotting this issue.

Will fix this and include in the next submission, unless you want to do it yourself.

Thanks
Jay

-----Original Message-----
From: Shlomo Pongratz [mailto:shlomop@mellanox.com] 
Sent: Monday, March 18, 2013 8:12 AM
To: linux-scsi@vger.kernel.org
Cc: Kallickal, Jayamohan; michaelc@cs.wisc.edu
Subject: Possible reentrancy issue in be_iopoll

Hi Jayamohan.

I think that there is a reentrancy issue in "drivers/scsi/be2iscsi/be_main.c::be_iopoll".
The driver creates "NAPI" context per core which is fine, however the above routine declares the ret variable as static!
Thus there is only one instance of this variable, so if this routine is called from more than one thread of execution the result is unpredictable.

         static unsigned int ret;
         .....

         ret = beiscsi_process_cq(pbe_eq); <----------------------------------------------------------------------------------Another
thread can enter here and change "ret".
         if (ret < budget) {
                ....
         }
<----------------------------------------------------------------------------------Another
thread can enter here and change "ret".
         return ret;

I can't submit a patch without checking it first, which is impossible since I don't have the HW, but the fix seems trivial, just take out the static.

Best regards,

S.P.