[PATCH 5.15.y] scsi: lpfc: Fix a possible data race in lpfc_unregister_fcf_rescan()

[PATCH 5.15.y] scsi: lpfc: Fix a possible data race in lpfc_unregister_fcf_rescan()

[bin.lan.cn]

From: Tuo Li <islituo@gmail.com>

[ Upstream commit 0e881c0a4b6146b7e856735226208f48251facd8 ]

The variable phba->fcf.fcf_flag is often protected by the lock
phba->hbalock() when is accessed. Here is an example in
lpfc_unregister_fcf_rescan():

  spin_lock_irq(&phba->hbalock);
  phba->fcf.fcf_flag |= FCF_INIT_DISC;
  spin_unlock_irq(&phba->hbalock);

However, in the same function, phba->fcf.fcf_flag is assigned with 0
without holding the lock, and thus can cause a data race:

  phba->fcf.fcf_flag = 0;

To fix this possible data race, a lock and unlock pair is added when
accessing the variable phba->fcf.fcf_flag.

Reported-by: BassCheck <bass@buaa.edu.cn>
Signed-off-by: Tuo Li <islituo@gmail.com>
Link: https://lore.kernel.org/r/20230630024748.1035993-1-islituo@gmail.com
Reviewed-by: Justin Tee <justin.tee@broadcom.com>
Reviewed-by: Laurence Oberman <loberman@redhat.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Bin Lan <bin.lan.cn@windriver.com>
Signed-off-by: He Zhe <zhe.he@windriver.com>
---
Build test passed.
---
 drivers/scsi/lpfc/lpfc_hbadisc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index 4bb0a15cfcc0..54aff304cdcf 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -6954,7 +6954,9 @@ lpfc_unregister_fcf_rescan(struct lpfc_hba *phba)
 	if (rc)
 		return;
 	/* Reset HBA FCF states after successful unregister FCF */
+	spin_lock_irq(&phba->hbalock);
 	phba->fcf.fcf_flag = 0;
+	spin_unlock_irq(&phba->hbalock);
 	phba->fcf.current_rec.flag = 0;
 
 	/*
-- 
2.34.1

[PATCH 5.10.y] scsi: lpfc: Fix a possible data race in lpfc_unregister_fcf_rescan()

[bin.lan.cn]

From: Tuo Li <islituo@gmail.com>

[ Upstream commit 0e881c0a4b6146b7e856735226208f48251facd8 ]

The variable phba->fcf.fcf_flag is often protected by the lock
phba->hbalock() when is accessed. Here is an example in
lpfc_unregister_fcf_rescan():

  spin_lock_irq(&phba->hbalock);
  phba->fcf.fcf_flag |= FCF_INIT_DISC;
  spin_unlock_irq(&phba->hbalock);

However, in the same function, phba->fcf.fcf_flag is assigned with 0
without holding the lock, and thus can cause a data race:

  phba->fcf.fcf_flag = 0;

To fix this possible data race, a lock and unlock pair is added when
accessing the variable phba->fcf.fcf_flag.

Reported-by: BassCheck <bass@buaa.edu.cn>
Signed-off-by: Tuo Li <islituo@gmail.com>
Link: https://lore.kernel.org/r/20230630024748.1035993-1-islituo@gmail.com
Reviewed-by: Justin Tee <justin.tee@broadcom.com>
Reviewed-by: Laurence Oberman <loberman@redhat.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Bin Lan <bin.lan.cn@windriver.com>
Signed-off-by: He Zhe <zhe.he@windriver.com>
---
Build test passed.
---
 drivers/scsi/lpfc/lpfc_hbadisc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index 68ff233f936e..3ff76ca147a5 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -6790,7 +6790,9 @@ lpfc_unregister_fcf_rescan(struct lpfc_hba *phba)
 	if (rc)
 		return;
 	/* Reset HBA FCF states after successful unregister FCF */
+	spin_lock_irq(&phba->hbalock);
 	phba->fcf.fcf_flag = 0;
+	spin_unlock_irq(&phba->hbalock);
 	phba->fcf.current_rec.flag = 0;
 
 	/*
-- 
2.34.1

Re: [PATCH 5.10.y] scsi: lpfc: Fix a possible data race in lpfc_unregister_fcf_rescan()

[Bin Lan]

There is a configuration dependency issue in the original 5.10.y, 
CONFIG_SCSI_LPFC should rely on CONFIG_IRQ_POLL.
So it is needed to enable CONFIG_IRQ_POLL also when building the changed 
source code. Otherwise you get a link error:
ld: drivers/scsi/lpfc/lpfc_sli.o: in function `__lpfc_sli4_process_cq':
lpfc_sli.c:(.text+0x37b6): undefined reference to `irq_poll_complete'
ld: drivers/scsi/lpfc/lpfc_sli.o: in function `lpfc_sli4_process_eq':
lpfc_sli.c:(.text+0x41b1): undefined reference to `irq_poll_sched'
ld: drivers/scsi/lpfc/lpfc_sli.o: in function `lpfc_cq_create':
lpfc_sli.c:(.text+0x1281f): undefined reference to `irq_poll_init'

--

Bin Lan

On 4/3/2025 11:29 AM, bin.lan.cn@windriver.com wrote:
> From: Tuo Li <islituo@gmail.com>
>
> [ Upstream commit 0e881c0a4b6146b7e856735226208f48251facd8 ]
>
> The variable phba->fcf.fcf_flag is often protected by the lock
> phba->hbalock() when is accessed. Here is an example in
> lpfc_unregister_fcf_rescan():
>
>    spin_lock_irq(&phba->hbalock);
>    phba->fcf.fcf_flag |= FCF_INIT_DISC;
>    spin_unlock_irq(&phba->hbalock);
>
> However, in the same function, phba->fcf.fcf_flag is assigned with 0
> without holding the lock, and thus can cause a data race:
>
>    phba->fcf.fcf_flag = 0;
>
> To fix this possible data race, a lock and unlock pair is added when
> accessing the variable phba->fcf.fcf_flag.
>
> Reported-by: BassCheck <bass@buaa.edu.cn>
> Signed-off-by: Tuo Li <islituo@gmail.com>
> Link: https://lore.kernel.org/r/20230630024748.1035993-1-islituo@gmail.com
> Reviewed-by: Justin Tee <justin.tee@broadcom.com>
> Reviewed-by: Laurence Oberman <loberman@redhat.com>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> Signed-off-by: Bin Lan <bin.lan.cn@windriver.com>
> Signed-off-by: He Zhe <zhe.he@windriver.com>
> ---
> Build test passed.
> ---
>   drivers/scsi/lpfc/lpfc_hbadisc.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
> index 68ff233f936e..3ff76ca147a5 100644
> --- a/drivers/scsi/lpfc/lpfc_hbadisc.c
> +++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
> @@ -6790,7 +6790,9 @@ lpfc_unregister_fcf_rescan(struct lpfc_hba *phba)
>   	if (rc)
>   		return;
>   	/* Reset HBA FCF states after successful unregister FCF */
> +	spin_lock_irq(&phba->hbalock);
>   	phba->fcf.fcf_flag = 0;
> +	spin_unlock_irq(&phba->hbalock);
>   	phba->fcf.current_rec.flag = 0;
>   
>   	/*

Re: [PATCH 5.15.y] scsi: lpfc: Fix a possible data race in lpfc_unregister_fcf_rescan()

[Sasha Levin]

[ Sasha's backport helper bot ]

Hi,

✅ All tests passed successfully. No issues detected.
No action required from the submitter.

The upstream commit SHA1 provided is correct: 0e881c0a4b6146b7e856735226208f48251facd8

WARNING: Author mismatch between patch and upstream commit:
Backport author: bin.lan.cn@windriver.com
Commit author: Tuo Li<islituo@gmail.com>

Status in newer kernel trees:
6.13.y | Present (exact SHA1)
6.12.y | Present (exact SHA1)
6.6.y | Present (exact SHA1)
6.1.y | Present (different SHA1: 30652c8ceb9a)

Note: The patch differs from the upstream commit:
---
1:  0e881c0a4b614 ! 1:  878d9878dbf20 scsi: lpfc: Fix a possible data race in lpfc_unregister_fcf_rescan()
    @@ Metadata
      ## Commit message ##
         scsi: lpfc: Fix a possible data race in lpfc_unregister_fcf_rescan()
     
    +    [ Upstream commit 0e881c0a4b6146b7e856735226208f48251facd8 ]
    +
         The variable phba->fcf.fcf_flag is often protected by the lock
         phba->hbalock() when is accessed. Here is an example in
         lpfc_unregister_fcf_rescan():
    @@ Commit message
         Reviewed-by: Justin Tee <justin.tee@broadcom.com>
         Reviewed-by: Laurence Oberman <loberman@redhat.com>
         Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
    +    Signed-off-by: Bin Lan <bin.lan.cn@windriver.com>
    +    Signed-off-by: He Zhe <zhe.he@windriver.com>
     
      ## drivers/scsi/lpfc/lpfc_hbadisc.c ##
     @@ drivers/scsi/lpfc/lpfc_hbadisc.c: lpfc_unregister_fcf_rescan(struct lpfc_hba *phba)
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-5.15.y       |  Success    |  Success   |

Re: [PATCH 5.10.y] scsi: lpfc: Fix a possible data race in lpfc_unregister_fcf_rescan()

[Sasha Levin]

[ Sasha's backport helper bot ]

Hi,

✅ All tests passed successfully. No issues detected.
No action required from the submitter.

The upstream commit SHA1 provided is correct: 0e881c0a4b6146b7e856735226208f48251facd8

WARNING: Author mismatch between patch and upstream commit:
Backport author: bin.lan.cn@windriver.com
Commit author: Tuo Li<islituo@gmail.com>

Status in newer kernel trees:
6.13.y | Present (exact SHA1)
6.12.y | Present (exact SHA1)
6.6.y | Present (exact SHA1)
6.1.y | Present (different SHA1: 30652c8ceb9a)
5.15.y | Not found

Note: The patch differs from the upstream commit:
---
1:  0e881c0a4b614 ! 1:  efb93eed6f136 scsi: lpfc: Fix a possible data race in lpfc_unregister_fcf_rescan()
    @@ Metadata
      ## Commit message ##
         scsi: lpfc: Fix a possible data race in lpfc_unregister_fcf_rescan()
     
    +    [ Upstream commit 0e881c0a4b6146b7e856735226208f48251facd8 ]
    +
         The variable phba->fcf.fcf_flag is often protected by the lock
         phba->hbalock() when is accessed. Here is an example in
         lpfc_unregister_fcf_rescan():
    @@ Commit message
         Reviewed-by: Justin Tee <justin.tee@broadcom.com>
         Reviewed-by: Laurence Oberman <loberman@redhat.com>
         Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
    +    Signed-off-by: Bin Lan <bin.lan.cn@windriver.com>
    +    Signed-off-by: He Zhe <zhe.he@windriver.com>
     
      ## drivers/scsi/lpfc/lpfc_hbadisc.c ##
     @@ drivers/scsi/lpfc/lpfc_hbadisc.c: lpfc_unregister_fcf_rescan(struct lpfc_hba *phba)
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-5.10.y       |  Success    |  Success   |

Re: [PATCH 5.10.y] scsi: lpfc: Fix a possible data race in lpfc_unregister_fcf_rescan()

[Justin Tee]

[-- Attachment #1: Type: text/plain, Size: 1077 bytes --]

Hi Greg,

> There is a configuration dependency issue in the original 5.10.y,
> CONFIG_SCSI_LPFC should rely on CONFIG_IRQ_POLL.
Is it possible to help out by porting this commit "fad0a16130b6 scsi:
lpfc: Add auto select on IRQ_POLL" into stable tree branches 5.9 and
later?

Thanks,
Justin

-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4190 bytes --]

Re: [PATCH 5.10.y] scsi: lpfc: Fix a possible data race in lpfc_unregister_fcf_rescan()

[Greg KH]

On Thu, Apr 03, 2025 at 01:58:00PM -0700, Justin Tee wrote:
> This electronic communication and the information and any files transmitted 
> with it, or attached to it, are confidential and are intended solely for 
> the use of the individual or entity to whom it is addressed and may contain 
> information that is confidential, legally privileged, protected by privacy 
> laws, or otherwise restricted from disclosure to anyone else. If you are 
> not the intended recipient or the person responsible for delivering the 
> e-mail to the intended recipient, you are hereby notified that any use, 
> copying, distributing, dissemination, forwarding, printing, or copying of 
> this e-mail is strictly prohibited. If you received this e-mail in error, 
> please return the e-mail to the sender, delete it from your computer, and 
> destroy any printed copy of it.

Now deleted.

Re: [PATCH 5.10.y] scsi: lpfc: Fix a possible data race in lpfc_unregister_fcf_rescan()

[Justin Tee]

Hi Greg,

> > This electronic communication and the information and any files transmitted
> > with it, or attached to it, are confidential and are intended solely for
> > the use of the individual or entity to whom it is addressed and may contain
> > information that is confidential, legally privileged, protected by privacy
> > laws, or otherwise restricted from disclosure to anyone else. If you are
> > not the intended recipient or the person responsible for delivering the
> > e-mail to the intended recipient, you are hereby notified that any use,
> > copying, distributing, dissemination, forwarding, printing, or copying of
> > this e-mail is strictly prohibited. If you received this e-mail in error,
> > please return the e-mail to the sender, delete it from your computer, and
> > destroy any printed copy of it.
>
> Now deleted.

You and everyone on this mailing list are intended recipients, so
there is no need to delete.

Please help port this commit to stable branches 5.9 and up to resolve
the compilation issue Bin reported.
https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/commit/?id=fad0a16130b6b4eb0958f4142d82509f90efdcbd

Thanks,
Justin

Re: [PATCH 5.10.y] scsi: lpfc: Fix a possible data race in lpfc_unregister_fcf_rescan()

[Greg KH]

On Fri, Apr 04, 2025 at 01:23:49PM -0700, Justin Tee wrote:
> Hi Greg,
> 
> > > This electronic communication and the information and any files transmitted
> > > with it, or attached to it, are confidential and are intended solely for
> > > the use of the individual or entity to whom it is addressed and may contain
> > > information that is confidential, legally privileged, protected by privacy
> > > laws, or otherwise restricted from disclosure to anyone else. If you are
> > > not the intended recipient or the person responsible for delivering the
> > > e-mail to the intended recipient, you are hereby notified that any use,
> > > copying, distributing, dissemination, forwarding, printing, or copying of
> > > this e-mail is strictly prohibited. If you received this e-mail in error,
> > > please return the e-mail to the sender, delete it from your computer, and
> > > destroy any printed copy of it.
> >
> > Now deleted.
> 
> You and everyone on this mailing list are intended recipients, so
> there is no need to delete.

Not true, you said this is "confidential" and according to our legal
advivice, that is not allowed to be on a change for a contribution to
Linux.  Please remove this type of message if you wish to contribute to
open source projects.

thanks,

greg k-h