[dpdk-dev] examples/vhost_scsi: replace strncpy with strlcpy

Message ID 1525865729-16086-3-git-send-email-reshma.pattan@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Pattan, Reshma May 9, 2018, 11:35 a.m. UTC
  Use strlcpy instead of strncpy.

Fixes: db75c7af19 ("examples/vhost_scsi: introduce a new sample app")
CC: stable@dpdk.org

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 examples/vhost_scsi/scsi.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
  

Comments

Bruce Richardson May 9, 2018, 1:37 p.m. UTC | #1
On Wed, May 09, 2018 at 12:35:29PM +0100, Reshma Pattan wrote:
> Use strlcpy instead of strncpy.
> 
> Fixes: db75c7af19 ("examples/vhost_scsi: introduce a new sample app")
> CC: stable@dpdk.org
> 
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> ---
>  examples/vhost_scsi/scsi.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/examples/vhost_scsi/scsi.c b/examples/vhost_scsi/scsi.c
> index 2a034bb9b..dd5be3c7e 100644
> --- a/examples/vhost_scsi/scsi.c
> +++ b/examples/vhost_scsi/scsi.c
> @@ -20,6 +20,7 @@
>  #include <rte_log.h>
>  #include <rte_malloc.h>
>  #include <rte_byteorder.h>
> +#include <rte_string_fns.h>
>  
>  #include "vhost_scsi.h"
>  #include "scsi_spec.h"
> @@ -181,7 +182,7 @@ vhost_bdev_scsi_inquiry_command(struct vhost_block_dev *bdev,
>  			break;
>  		case SPC_VPD_UNIT_SERIAL_NUMBER:
>  			hlen = 4;
> -			strncpy((char *)vpage->params, bdev->name, 32);
> +			strlcpy((char *)vpage->params, bdev->name, 32);
>  			vpage->alloc_len = rte_cpu_to_be_16(32);
>  			break;
>  		case SPC_VPD_DEVICE_IDENTIFICATION:
> @@ -215,10 +216,10 @@ vhost_bdev_scsi_inquiry_command(struct vhost_block_dev *bdev,
>  			desig->piv = 1;
>  			desig->reserved1 = 0;
>  			desig->len = 8 + 16 + 32;
> -			strncpy((char *)desig->desig, "INTEL", 8);
> +			strlcpy((char *)desig->desig, "INTEL", 8);
>  			vhost_strcpy_pad((char *)&desig->desig[8],
>  					 bdev->product_name, 16, ' ');
> -			strncpy((char *)&desig->desig[24], bdev->name, 32);
> +			strlcpy((char *)&desig->desig[24], bdev->name, 32);
>  			len += sizeof(struct scsi_desig_desc) + 8 + 16 + 32;
>  
>  			buf += sizeof(struct scsi_desig_desc) + desig->len;
> @@ -275,7 +276,7 @@ vhost_bdev_scsi_inquiry_command(struct vhost_block_dev *bdev,
>  		inqdata->flags3 = 0x2;
>  
>  		/* T10 VENDOR IDENTIFICATION */
> -		strncpy((char *)inqdata->t10_vendor_id, "INTEL", 8);
> +		strlcpy((char *)inqdata->t10_vendor_id, "INTEL", 8);
>  
>  		/* PRODUCT IDENTIFICATION */
>  		snprintf((char *)inqdata->product_id,
> @@ -283,7 +284,7 @@ vhost_bdev_scsi_inquiry_command(struct vhost_block_dev *bdev,
>  				bdev->product_name);
>  
>  		/* PRODUCT REVISION LEVEL */
> -		strncpy((char *)inqdata->product_rev, "0001", 4);
> +		strlcpy((char *)inqdata->product_rev, "0001", 4);
>  
>  		/* Standard inquiry data ends here. Only populate
>  		 * remaining fields if alloc_len indicates enough
> -- 
Can the magic numbers "32", "8", "4" be replaced with non-magic values,
e.g. sizeof(...).

/Bruce
  
Pattan, Reshma May 9, 2018, 4:38 p.m. UTC | #2
Hi Bruce,

> -----Original Message-----
> From: Richardson, Bruce
> Sent: Wednesday, May 9, 2018 2:37 PM
> To: Pattan, Reshma <reshma.pattan@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] examples/vhost_scsi: replace strncpy with
> strlcpy
> 
> On Wed, May 09, 2018 at 12:35:29PM +0100, Reshma Pattan wrote:
> > Use strlcpy instead of strncpy.
> >
> > Fixes: db75c7af19 ("examples/vhost_scsi: introduce a new sample app")
> > CC: stable@dpdk.org
> >
> > Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> > ---
> > -			strncpy((char *)vpage->params, bdev->name, 32);
> > +			strlcpy((char *)vpage->params, bdev->name, 32);
> > -			strncpy((char *)desig->desig, "INTEL", 8);
> > +			strlcpy((char *)desig->desig, "INTEL", 8);
> >  			vhost_strcpy_pad((char *)&desig->desig[8],
> >  					 bdev->product_name, 16, ' ');
> > -			strncpy((char *)&desig->desig[24], bdev->name, 32);
> > +			strlcpy((char *)&desig->desig[24], bdev->name, 32);
> >
> >
> > --
> Can the magic numbers "32", "8", "4" be replaced with non-magic values, e.g.
> sizeof(...).
> 
If I take below piece of code as example ,  the application is trying to copy 
below strings to bare array which has no size defined to it. 
Neither there are any variable  like brand name or device name available in desig object which of type `struct scsi_desig_desc`,  which I can use to pass on to sizeof()  for using it for strlcpy. So not sure how to do address the comment of using sizeof(). Any suggestions?

    strlcpy((char *)desig->desig, "INTEL", 8);
    strlcpy((char *)&desig->desig[24], bdev->name, 32);

/* designation descriptor */
struct scsi_desig_desc {
uint8_t code_set>*******: 4;
uint8_t protocol_id>****: 4;
uint8_t type>***>*******: 4;
uint8_t association>****: 2;
uint8_t reserved0>******: 1;
uint8_t piv>****>*******: 1;
uint8_t reserved1;
uint8_t>len;
uint8_t desig[];
};

Thanks,
Reshma
  
Pattan, Reshma May 10, 2018, 12:24 p.m. UTC | #3
Hi Bruce,

> Can the magic numbers "32", "8", "4" be replaced with non-magic values, e.g.
> sizeof(...).
> 

This is now taken care wherever possible in latest patch.

Thanks,
Reshma

> /Bruce
  

Patch

diff --git a/examples/vhost_scsi/scsi.c b/examples/vhost_scsi/scsi.c
index 2a034bb9b..dd5be3c7e 100644
--- a/examples/vhost_scsi/scsi.c
+++ b/examples/vhost_scsi/scsi.c
@@ -20,6 +20,7 @@ 
 #include <rte_log.h>
 #include <rte_malloc.h>
 #include <rte_byteorder.h>
+#include <rte_string_fns.h>
 
 #include "vhost_scsi.h"
 #include "scsi_spec.h"
@@ -181,7 +182,7 @@  vhost_bdev_scsi_inquiry_command(struct vhost_block_dev *bdev,
 			break;
 		case SPC_VPD_UNIT_SERIAL_NUMBER:
 			hlen = 4;
-			strncpy((char *)vpage->params, bdev->name, 32);
+			strlcpy((char *)vpage->params, bdev->name, 32);
 			vpage->alloc_len = rte_cpu_to_be_16(32);
 			break;
 		case SPC_VPD_DEVICE_IDENTIFICATION:
@@ -215,10 +216,10 @@  vhost_bdev_scsi_inquiry_command(struct vhost_block_dev *bdev,
 			desig->piv = 1;
 			desig->reserved1 = 0;
 			desig->len = 8 + 16 + 32;
-			strncpy((char *)desig->desig, "INTEL", 8);
+			strlcpy((char *)desig->desig, "INTEL", 8);
 			vhost_strcpy_pad((char *)&desig->desig[8],
 					 bdev->product_name, 16, ' ');
-			strncpy((char *)&desig->desig[24], bdev->name, 32);
+			strlcpy((char *)&desig->desig[24], bdev->name, 32);
 			len += sizeof(struct scsi_desig_desc) + 8 + 16 + 32;
 
 			buf += sizeof(struct scsi_desig_desc) + desig->len;
@@ -275,7 +276,7 @@  vhost_bdev_scsi_inquiry_command(struct vhost_block_dev *bdev,
 		inqdata->flags3 = 0x2;
 
 		/* T10 VENDOR IDENTIFICATION */
-		strncpy((char *)inqdata->t10_vendor_id, "INTEL", 8);
+		strlcpy((char *)inqdata->t10_vendor_id, "INTEL", 8);
 
 		/* PRODUCT IDENTIFICATION */
 		snprintf((char *)inqdata->product_id,
@@ -283,7 +284,7 @@  vhost_bdev_scsi_inquiry_command(struct vhost_block_dev *bdev,
 				bdev->product_name);
 
 		/* PRODUCT REVISION LEVEL */
-		strncpy((char *)inqdata->product_rev, "0001", 4);
+		strlcpy((char *)inqdata->product_rev, "0001", 4);
 
 		/* Standard inquiry data ends here. Only populate
 		 * remaining fields if alloc_len indicates enough