[PATCH 0/3] scsi: check set_identify result in update_links

[PATCH 0/3] scsi: check set_identify result in update_links

[Shirong Zhao]

update_links() in mpt3sas, mpi3mr and leapraid all ignore the return
value of their set_identify helper.  On firmware query failure the
identify struct is left untouched, and the phy is then attached using
a stale sas_address from before the failed query.

This series fixes all three the same way: fold the call into the if
condition so a failed query takes the existing zeroing else path, the
same "no valid identify" state as an invalid handle or link rate.
The next successful link event re-fills the identify.

All three were verified against scsi.git fixes (469fa055664b): each
applies cleanly, passes checkpatch --strict with no warnings, and
its transport object builds warning-free before and after the change.

Shirong Zhao (3):
  scsi: mpt3sas: check set_identify result in update_links
  scsi: mpi3mr: check set_identify result in update_links
  scsi: leapraid: check set_identify result in update_links

 drivers/scsi/leapraid/leapraid_transport.c | 6 +++---
 drivers/scsi/mpi3mr/mpi3mr_transport.c     | 6 +++---
 drivers/scsi/mpt3sas/mpt3sas_transport.c   | 6 +++---
 3 files changed, 9 insertions(+), 9 deletions(-)

[PATCH 1/3] scsi: mpt3sas: check set_identify result in update_links

[Shirong Zhao]

mpt3sas_transport_update_links() ignores the return value of
_transport_set_identify(), which leaves the identify struct untouched
on FW query failure (host/PCI recovery, config page error, bad IOC
status).  The phy is then attached using a stale
remote_identify.sas_address from before the failed query.

Fold the call into the if condition so a failed query takes the
existing zeroing else path (same "no valid identify" state as an
invalid handle/rate) instead of attaching with stale data.  The next
successful link event re-fills the identify.

Verified against scsi.git fixes (469fa055664b), the current SCSI
fixes tree.

Signed-off-by: Shirong Zhao <shxzhaosr@163.com>
---
 drivers/scsi/mpt3sas/mpt3sas_transport.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_transport.c b/drivers/scsi/mpt3sas/mpt3sas_transport.c
index e74a526ef..ffd1d37d2 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_transport.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_transport.c
@@ -1179,9 +1179,9 @@ mpt3sas_transport_update_links(struct MPT3SAS_ADAPTER *ioc,
 	mpt3sas_phy = &sas_node->phy[phy_number];
 	mpt3sas_phy->attached_handle = handle;
 	spin_unlock_irqrestore(&ioc->sas_node_lock, flags);
-	if (handle && (link_rate >= MPI2_SAS_NEG_LINK_RATE_1_5)) {
-		_transport_set_identify(ioc, handle,
-		    &mpt3sas_phy->remote_identify);
+	if (handle && link_rate >= MPI2_SAS_NEG_LINK_RATE_1_5 &&
+	    !_transport_set_identify(ioc, handle,
+				    &mpt3sas_phy->remote_identify)) {
 		if ((sas_node->handle <= ioc->sas_hba.num_phys) &&
 		    (ioc->multipath_on_hba)) {
 			list_for_each_entry(hba_port,

[PATCH 2/3] scsi: mpi3mr: check set_identify result in update_links

[Shirong Zhao]

mpi3mr_update_links() ignores the return value of
mpi3mr_set_identify(), which leaves the identify struct untouched
on FW query failure (recovery in progress, config page error, bad
IOC status).  The phy is then attached using a stale
mr_sas_phy->remote_identify.sas_address from before the failed query.

Fold the call into the if condition so a failed query takes the
existing zeroing else path (same "no valid identify" state as an
invalid handle/rate) instead of attaching with stale data.  The next
successful link event re-fills the identify.

Verified against scsi.git fixes (469fa055664b), the current SCSI
fixes tree.

Signed-off-by: Shirong Zhao <shxzhaosr@163.com>
---
 drivers/scsi/mpi3mr/mpi3mr_transport.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_transport.c b/drivers/scsi/mpi3mr/mpi3mr_transport.c
index 232af978d..abe8c4fb9 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_transport.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_transport.c
@@ -1093,9 +1093,9 @@ void mpi3mr_update_links(struct mpi3mr_ioc *mrioc,
 	mr_sas_phy = &mr_sas_node->phy[phy_number];
 	mr_sas_phy->attached_handle = handle;
 	spin_unlock_irqrestore(&mrioc->sas_node_lock, flags);
-	if (handle && (link_rate >= MPI3_SAS_NEG_LINK_RATE_1_5)) {
-		mpi3mr_set_identify(mrioc, handle,
-		    &mr_sas_phy->remote_identify);
+	if (handle && link_rate >= MPI3_SAS_NEG_LINK_RATE_1_5 &&
+	    !mpi3mr_set_identify(mrioc, handle,
+				 &mr_sas_phy->remote_identify)) {
 		mpi3mr_add_phy_to_an_existing_port(mrioc, mr_sas_node,
 		    mr_sas_phy, mr_sas_phy->remote_identify.sas_address,
 		    hba_port);

[PATCH 3/3] scsi: leapraid: check set_identify result in update_links

[Shirong Zhao]

leapraid_transport_update_links() ignores the return value of
leapraid_transport_set_identify(), which leaves the identify struct
untouched on FW query failure (recovery in progress, config page
error).  The phy is then attached using a stale
card_phy->remote_identify.sas_address from before the failed query.

Fold the call into the if condition so a failed query takes the
existing zeroing else path (same "no valid identify" state as an
invalid handle/rate) instead of attaching with stale data.  The next
successful link event re-fills the identify.

Verified against scsi.git fixes (469fa055664b), the current SCSI
fixes tree.

Signed-off-by: Shirong Zhao <shxzhaosr@163.com>
---
 drivers/scsi/leapraid/leapraid_transport.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/leapraid/leapraid_transport.c b/drivers/scsi/leapraid/leapraid_transport.c
index 77ffbd6dd..38e63327d 100644
--- a/drivers/scsi/leapraid/leapraid_transport.c
+++ b/drivers/scsi/leapraid/leapraid_transport.c
@@ -1150,9 +1150,9 @@ void leapraid_transport_update_links(
 	card_phy->attached_hdl = hdl;
 	spin_unlock_irqrestore(&adapter->dev_topo.topo_node_lock, flags);
 
-	if (hdl && link_rate >= LEAPRAID_SAS_NEG_LINK_RATE_1_5) {
-		leapraid_transport_set_identify(adapter, hdl,
-						&card_phy->remote_identify);
+	if (hdl && link_rate >= LEAPRAID_SAS_NEG_LINK_RATE_1_5 &&
+	    !leapraid_transport_set_identify(adapter, hdl,
+					     &card_phy->remote_identify)) {
 		if (topo_node->hdl <= adapter->dev_topo.card.phys_num &&
 		    adapter->adapter_attr.enable_mp) {
 			list_for_each_entry(card_port,