Skip to contentSkip to conversation

Possible reentrancy issue in be_iopoll

DISCUSSION Shlomo Pongratz 2 participants 2 messages Mar 18, 2013 – Mar 19, 2013 SCSI

lore
Share & tools
lore
#0 lore raw

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.

#1 lore raw

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:[email protected]]
Sent: Monday, March 18, 2013 8:12 AM
To: [email protected]
Cc: Kallickal, Jayamohan; [email protected]
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.