diff mbox series

[next] scsi: pm80xx: Fix potential infinite loop

Message ID 20210407135840.494747-1-colin.king@canonical.com (mailing list archive)
State New
Headers show
Series [next] scsi: pm80xx: Fix potential infinite loop | expand

Commit Message

Colin Ian King April 7, 2021, 1:58 p.m. UTC
From: Colin Ian King <colin.king@canonical.com>

The for-loop iterates with a u8 loop counter i and compares this
with the loop upper limit of pm8001_ha->max_q_num which is a u32
type.  There is a potential infinite loop if pm8001_ha->max_q_num
is larger than the u8 loop counter. Fix this by making the loop
counter the same type as pm8001_ha->max_q_num.

Addresses-Coverity: ("Infinite loop")
Fixes: 65df7d1986a1 ("scsi: pm80xx: Fix chip initialization failure")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/scsi/pm8001/pm8001_hwi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Johannes Thumshirn April 7, 2021, 2:13 p.m. UTC | #1
On 07/04/2021 15:58, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> The for-loop iterates with a u8 loop counter i and compares this
> with the loop upper limit of pm8001_ha->max_q_num which is a u32
> type.  There is a potential infinite loop if pm8001_ha->max_q_num
> is larger than the u8 loop counter. Fix this by making the loop
> counter the same type as pm8001_ha->max_q_num.

Heh, coincidentally I've read your blog post on this issue today.

> Addresses-Coverity: ("Infinite loop")
> Fixes: 65df7d1986a1 ("scsi: pm80xx: Fix chip initialization failure")

AFAICS this still is in Martin's tree and not yet in Linus' tree. 

Anyways, looks good.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Martin K. Petersen April 7, 2021, 5:18 p.m. UTC | #2
Hi Colin!

> The for-loop iterates with a u8 loop counter i and compares this with
> the loop upper limit of pm8001_ha->max_q_num which is a u32 type.
> There is a potential infinite loop if pm8001_ha->max_q_num is larger
> than the u8 loop counter. Fix this by making the loop counter the same
> type as pm8001_ha->max_q_num.

No particular objections to the patch for future-proofing. However, as
far as I can tell max_q_num is capped at 64 (PM8001_MAX_MSIX_VEC).
Jinpu Wang April 8, 2021, 5:21 a.m. UTC | #3
On Wed, Apr 7, 2021 at 7:18 PM Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>
>
> Hi Colin!
>
> > The for-loop iterates with a u8 loop counter i and compares this with
> > the loop upper limit of pm8001_ha->max_q_num which is a u32 type.
> > There is a potential infinite loop if pm8001_ha->max_q_num is larger
> > than the u8 loop counter. Fix this by making the loop counter the same
> > type as pm8001_ha->max_q_num.
>
> No particular objections to the patch for future-proofing. However, as
> far as I can tell max_q_num is capped at 64 (PM8001_MAX_MSIX_VEC).
Exactly.
>
> --
> Martin K. Petersen      Oracle Linux Engineering
diff mbox series

Patch

diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index d048455f4941..16edd84e7130 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -643,7 +643,7 @@  static void init_pci_device_addresses(struct pm8001_hba_info *pm8001_ha)
  */
 static int pm8001_chip_init(struct pm8001_hba_info *pm8001_ha)
 {
-	u8 i = 0;
+	u32 i = 0;
 	u16 deviceid;
 	pci_read_config_word(pm8001_ha->pdev, PCI_DEVICE_ID, &deviceid);
 	/* 8081 controllers need BAR shift to access MPI space