Skip to contentSkip to conversation

scsi: qla2xxx: edif: fix off by one bug in qla_edif_app_getfcinfo()

PATCH Applied Dan Carpenter 4 participants 4 messages Nov 9, 2021 – Nov 19, 2021

Subsystems (3)
SCSI Kernel janitors QLogic QLA2XXX FC-SCSI driver
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 scsi: qla2xxx: edif: fix off by one bug in qla_edif_app_getfcinfo()

The > comparison needs to be >= to prevent accessing one element beyond the end of the app_reply->ports[] array.

Fixes 7878f22a2e03 ("scsi: qla2xxx: edif: Add getfcinfo and statistic bsgs")
Signed-off-by Dan Carpenter <[email protected]>
---
 drivers/scsi/qla2xxx/qla_edif.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/scsi/qla2xxx/qla_edif.c +1 -1
index 2e37b189cb75..53d2b8562027 100644
@@ -865,7 +865,7 @@ qla_edif_app_getfcinfo(scsi_qla_host_t *vha, struct bsg_job *bsg_job)
 			    "APP request entry - portid=%06x.\n", tdid.b24);
 
 			/* Ran out of space */
-			if (pcnt > app_req.num_ports)
+			if (pcnt >= app_req.num_ports)
 				break;
 
 			if (tdid.b24 != 0 && tdid.b24 != fcport->d_id.b24)
Signature · 1 line
2.20.1
Reviewed-by on v1 1/1
#1 lore raw

Looks fine. (The break; test could perhaps be moved prior to the ql_dbg() call above
but that's not all that critical. Or, that ql_dbg()) call could potentially move outside the list
traversal since it is invariant, Marvell you might want to look at that.)

Reviewed-by Ewan D. Milne <[email protected]>
Quoted 26 lines · Dan Carpenter

The > comparison needs to be >= to prevent accessing one element beyond the end of the app_reply->ports[] array.

Fixes 7878f22a2e03 ("scsi: qla2xxx: edif: Add getfcinfo and statistic bsgs")
Signed-off-by Dan Carpenter <[email protected]>
---
 drivers/scsi/qla2xxx/qla_edif.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/scsi/qla2xxx/qla_edif.c +1 -2
index 2e37b189cb75..53d2b8562027 100644
@@ -865,7 +865,7 @@ qla_edif_app_getfcinfo(scsi_qla_host_t *vha, struct bsg_job *bsg_job)
                            "APP request entry - portid=%06x.\n", tdid.b24);
 
                        /* Ran out of space */
-                       if (pcnt > app_req.num_ports)
+                       if (pcnt >= app_req.num_ports)
                                break;
 
                        if (tdid.b24 != 0 && tdid.b24 != fcport->d_id.b24)
--

2.20.1

Reviewed-by on v1 1/1
#2 lore raw
In reply to Dan Carpenter
Quoted 29 lines · Dan Carpenter

The > comparison needs to be >= to prevent accessing one element beyond the end of the app_reply->ports[] array.

Fixes 7878f22a2e03 ("scsi: qla2xxx: edif: Add getfcinfo and statistic bsgs")
Signed-off-by Dan Carpenter <[email protected]>

---
drivers/scsi/qla2xxx/qla_edif.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
drivers/scsi/qla2xxx/qla_edif.c +1 -1
index 2e37b189cb75..53d2b8562027 100644
@@ -865,7 +865,7 @@ qla_edif_app_getfcinfo(scsi_qla_host_t *vha, struct bsg_job *bsg_job)
 			    "APP request entry - portid=%06x.\n", tdid.b24);
 
 			/* Ran out of space */
-			if (pcnt > app_req.num_ports)
+			if (pcnt >= app_req.num_ports)
 				break;
 
 			if (tdid.b24 != 0 && tdid.b24 != fcport->d_id.b24)
Signature · 1 line
2.20.1
Reviewed-by Himanshu Madhani <[email protected]>

Also, I agree with comments from Ewan about the ql_dbg(), it needs to be fixed as separate patch.

--
Himanshu Madhani Oracle Linux Engineering