[patch] off by one in FlashPoint.c

[patch] off by one in FlashPoint.c

[Dan Carpenter]

The check should be >= instead of > or we could go past the end of the
array.

Signed-off-by: Dan Carpenter <error27@gmail.com>

--- orig/drivers/scsi/FlashPoint.c	2009-12-27 11:35:58.000000000 +0200
+++ devel/drivers/scsi/FlashPoint.c	2009-12-27 11:36:19.000000000 +0200
@@ -3924,7 +3924,7 @@
 {
 	struct sccb_mgr_tar_info *currTar_Info;
 
-	if ((p_sccb->TargID > MAX_SCSI_TAR) || (p_sccb->Lun > MAX_LUN)) {
+	if ((p_sccb->TargID >= MAX_SCSI_TAR) || (p_sccb->Lun > MAX_LUN)) {
 		return;
 	}
 	currTar_Info = &FPT_sccbMgrTbl[p_card][p_sccb->TargID];

Re: [patch] off by one in FlashPoint.c

[Joe Eykholt]

Dan Carpenter wrote:
> The check should be >= instead of > or we could go past the end of the
> array.

What about the LUN check?   I'm not sure either way, but usually MAX LUN
is the max number of LUNs, not the max LU number, although that's been
in flux lately.

> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> 
> --- orig/drivers/scsi/FlashPoint.c	2009-12-27 11:35:58.000000000 +0200
> +++ devel/drivers/scsi/FlashPoint.c	2009-12-27 11:36:19.000000000 +0200
> @@ -3924,7 +3924,7 @@
>  {
>  	struct sccb_mgr_tar_info *currTar_Info;
>  
> -	if ((p_sccb->TargID > MAX_SCSI_TAR) || (p_sccb->Lun > MAX_LUN)) {
> +	if ((p_sccb->TargID >= MAX_SCSI_TAR) || (p_sccb->Lun > MAX_LUN)) {
>  		return;
>  	}
>  	currTar_Info = &FPT_sccbMgrTbl[p_card][p_sccb->TargID];
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] off by one in FlashPoint.c

[Dan Carpenter]

On Mon, Dec 28, 2009 at 12:14:39PM -0500, Joe Eykholt wrote:
>
>
> Dan Carpenter wrote:
>> The check should be >= instead of > or we could go past the end of the
>> array.
>
> What about the LUN check?   I'm not sure either way, but usually MAX LUN
> is the max number of LUNs, not the max LU number, although that's been
> in flux lately.
>

No no.  You're clearly correct.  That matches how MAX_LUN is used throughout 
the file.

Will send an updated patch.

regards,
dan carpenter

>>
>> Signed-off-by: Dan Carpenter <error27@gmail.com>
>>
>> --- orig/drivers/scsi/FlashPoint.c	2009-12-27 11:35:58.000000000 +0200
>> +++ devel/drivers/scsi/FlashPoint.c	2009-12-27 11:36:19.000000000 +0200
>> @@ -3924,7 +3924,7 @@
>>  {
>>  	struct sccb_mgr_tar_info *currTar_Info;
>>  -	if ((p_sccb->TargID > MAX_SCSI_TAR) || (p_sccb->Lun > MAX_LUN)) {
>> +	if ((p_sccb->TargID >= MAX_SCSI_TAR) || (p_sccb->Lun > MAX_LUN)) {
>>  		return;
>>  	}
>>  	currTar_Info = &FPT_sccbMgrTbl[p_card][p_sccb->TargID];
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[patch] off by one in FlashPoint.c (v2)

[Dan Carpenter]

The check on MAX_SCSI_TAR should be >= instead of > or we could go past the 
end of the array.

Joe Eykholt aslo correctly points out that the check on MAX_LUN should be
>= as well.  That matches with how it is used in the rest of the file.

Signed-off-by:  Dan Carpenter <error27@gmail.com>

--- orig/drivers/scsi/FlashPoint.c	2009-12-27 11:35:58.000000000 +0200
+++ devel/drivers/scsi/FlashPoint.c	2009-12-28 19:59:33.000000000 +0200
@@ -3924,7 +3924,7 @@ static void FPT_sinits(struct sccb *p_sc
 {
 	struct sccb_mgr_tar_info *currTar_Info;
 
-	if ((p_sccb->TargID > MAX_SCSI_TAR) || (p_sccb->Lun > MAX_LUN)) {
+	if ((p_sccb->TargID >= MAX_SCSI_TAR) || (p_sccb->Lun >= MAX_LUN)) {
 		return;
 	}
 	currTar_Info = &FPT_sccbMgrTbl[p_card][p_sccb->TargID];