RE: [patch] bfa: clean up some bounds checking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



    Both the approaches (i.e., using a loop or the strchr()) look fine to me.

Thanks,
Sudarsana

-----Original Message-----
From: walter harms [mailto:wharms@xxxxxx] 
Sent: 16 June 2016 17:50
To: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Cc: Anil Gurumurthy <Anil.Gurumurthy@xxxxxxxxxx>; Sudarsana Kalluru <Sudarsana.Kalluru@xxxxxxxxxx>; James E.J. Bottomley <jejb@xxxxxxxxxxxxxxxxxx>; Martin K. Petersen <martin.petersen@xxxxxxxxxx>; linux-scsi <linux-scsi@xxxxxxxxxxxxxxx>; kernel-janitors@xxxxxxxxxxxxxxx
Subject: Re: [patch] bfa: clean up some bounds checking



Am 16.06.2016 12:44, schrieb Dan Carpenter:
> This code is supposed to search ->adapter_hwpath[] and replace the 
> second colon with a NUL character.  Unfortunately, the boundary checks 
> that ensure we don't go beyond the end of the buffer have a couple 
> problems.
> 
> Imagine that the string has no colons.  In that case, in the first 
> loop, we read one space beyond the end of the buffer and then exit the loop.
> In the next loop, we increment once, read two characters beyond the 
> end of the buffer and then exit.  Then after the loop we put a NUL 
> character two characters past the end of the buffer.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> ---
> This is from static analysis and not tested.  Caveat emptor.
> 
> diff --git a/drivers/scsi/bfa/bfad_bsg.c b/drivers/scsi/bfa/bfad_bsg.c 
> index d1ad020..dfb26f0 100644
> --- a/drivers/scsi/bfa/bfad_bsg.c
> +++ b/drivers/scsi/bfa/bfad_bsg.c
> @@ -106,10 +106,17 @@ bfad_iocmd_ioc_get_info(struct bfad_s *bfad, 
> void *cmd)
>  
>  	/* set adapter hw path */
>  	strcpy(iocmd->adapter_hwpath, bfad->pci_name);
> -	for (i = 0; iocmd->adapter_hwpath[i] != ':' && i < BFA_STRING_32; i++)
> -		;
> -	for (; iocmd->adapter_hwpath[++i] != ':' && i < BFA_STRING_32; )
> -		;
> +	i = -1;
> +	while (++i < BFA_STRING_32) {
> +		if (iocmd->adapter_hwpath[i] == ':')
> +			break;
> +	}
> +	while (++i < BFA_STRING_32) {
> +		if (iocmd->adapter_hwpath[i] == ':')
> +			break;
> +	}
> +	if (i >= BFA_STRING_32)
> +		i = BFA_STRING_32 - 1;
>  	iocmd->adapter_hwpath[i] = '\0';
>  	iocmd->status = BFA_STATUS_OK;
>  	return 0;


I do not see the use case but i assume
the idea is to have a string like aa:bb:something and kill everyhing after the second ':' ?
/*
	a few word may help here also inside the code */


second: maybe we can us strchr here ?
	s1=strchr(iocmd->adapter_hwpath,':');
	if (s1 != NULL ) s1=strchr(s1,":");



re,
 wh
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux