[PATCH] examples/ipsec-secgw: replace strncpy with strlcpy

[PATCH] examples/ipsec-secgw: replace strncpy with strlcpy

[Reshma Pattan]

Use strlcpy instead of strncpy.

Fixes: 0d547ed037 ("examples/ipsec-secgw: support configuration file")
Fixes: 07b156199f ("examples/ipsec-secgw: fix configuration string termination")
Fixes: a1469c319f ("examples/ipsec-secgw: fix configuration parsing")
Cc: stable@dpdk.org
CC: Zhang,Roy Fan <roy.fan.zhang@intel.com>

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 examples/ipsec-secgw/parser.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/examples/ipsec-secgw/parser.c b/examples/ipsec-secgw/parser.c
index 2403b564d..9ccd5ea72 100644
--- a/examples/ipsec-secgw/parser.c
+++ b/examples/ipsec-secgw/parser.c
@@ -3,6 +3,7 @@
  */
 #include <rte_common.h>
 #include <rte_crypto.h>
+#include <rte_string_fns.h>
 
 #include <cmdline_parse_string.h>
 #include <cmdline_parse_num.h>
@@ -212,14 +213,14 @@ parse_ipv4_addr(const char *token, struct in_addr *ipv4, uint32_t *mask)
 
 	pch = strchr(token, '/');
 	if (pch != NULL) {
-		strncpy(ip_str, token, pch - token);
+		strlcpy(ip_str, token, pch - token);
 		pch += 1;
 		if (is_str_num(pch) != 0)
 			return -EINVAL;
 		if (mask)
 			*mask = atoi(pch);
 	} else {
-		strncpy(ip_str, token, sizeof(ip_str) - 1);
+		strlcpy(ip_str, token, sizeof(ip_str) - 1);
 		if (mask)
 			*mask = 0;
 	}
@@ -241,14 +242,14 @@ parse_ipv6_addr(const char *token, struct in6_addr *ipv6, uint32_t *mask)
 
 	pch = strchr(token, '/');
 	if (pch != NULL) {
-		strncpy(ip_str, token, pch - token);
+		strlcpy(ip_str, token, pch - token);
 		pch += 1;
 		if (is_str_num(pch) != 0)
 			return -EINVAL;
 		if (mask)
 			*mask = atoi(pch);
 	} else {
-		strncpy(ip_str, token, sizeof(ip_str) - 1);
+		strlcpy(ip_str, token, sizeof(ip_str) - 1);
 		if (mask)
 			*mask = 0;
 	}
@@ -515,7 +516,7 @@ parse_cfg_file(const char *cfg_filename)
 				goto error_exit;
 			}
 
-			strncpy(str + strlen(str), oneline,
+			strlcpy(str + strlen(str), oneline,
 				strlen(oneline));
 
 			continue;
@@ -528,7 +529,7 @@ parse_cfg_file(const char *cfg_filename)
 				cfg_filename, line_num);
 			goto error_exit;
 		}
-		strncpy(str + strlen(str), oneline,
+		strlcpy(str + strlen(str), oneline,
 			strlen(oneline));
 
 		str[strlen(str)] = '\n';
-- 
2.14.3

[PATCH] examples/quota_watermark: fix gcc 8.0.1 cast between incompatible types

[Reshma Pattan]

Gcc 8.0.1 reports incompatible cast between types i.e. from
`void (*)(void *)` to `(int (*)(void *)`.

Change the pipeline_stage prototype to retun int type
to fix the issue.

Fixes: a0ffcb257a ("examples/quota_watermark: correct code indentation")
CC: stable@dpdk.org

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 examples/quota_watermark/qw/main.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/examples/quota_watermark/qw/main.c b/examples/quota_watermark/qw/main.c
index 313216f39..dda3e0a3e 100644
--- a/examples/quota_watermark/qw/main.c
+++ b/examples/quota_watermark/qw/main.c
@@ -181,7 +181,7 @@ receive_stage(__attribute__((unused)) void *args)
 	}
 }
 
-static void
+static int
 pipeline_stage(__attribute__((unused)) void *args)
 {
 	int i, ret;
@@ -243,9 +243,11 @@ pipeline_stage(__attribute__((unused)) void *args)
 			}
 		}
 	}
+
+	return 0;
 }
 
-static void
+static int
 send_stage(__attribute__((unused)) void *args)
 {
 	uint16_t nb_dq_pkts;
@@ -287,6 +289,8 @@ send_stage(__attribute__((unused)) void *args)
 			/* TODO: Check if nb_dq_pkts == nb_tx_pkts? */
 		}
 	}
+
+	return 0;
 }
 
 int
@@ -347,7 +351,7 @@ main(int argc, char **argv)
 					init_ring(lcore_id, port_id);
 
 			/* typecast is a workaround for GCC 4.3 bug */
-			rte_eal_remote_launch((int (*)(void *))pipeline_stage,
+			rte_eal_remote_launch(pipeline_stage,
 					NULL, lcore_id);
 		}
 	}
-- 
2.14.3

Re: [dpdk-stable] [PATCH] examples/quota_watermark: fix gcc 8.0.1 cast between incompatible types

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: stable [mailto:stable-bounces@dpdk.org] On Behalf Of Reshma Pattan
> Sent: Wednesday, May 9, 2018 12:35 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Pattan, Reshma <reshma.pattan@intel.com>
> Subject: [dpdk-stable] [PATCH] examples/quota_watermark: fix gcc 8.0.1 cast
> between incompatible types
> 
> Gcc 8.0.1 reports incompatible cast between types i.e. from `void (*)(void *)` to
> `(int (*)(void *)`.
> 
> Change the pipeline_stage prototype to retun int type to fix the issue.
> 
> Fixes: a0ffcb257a ("examples/quota_watermark: correct code indentation")
> CC: stable@dpdk.org
> 
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>

Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

Re: [dpdk-stable] [PATCH] examples/quota_watermark: fix gcc 8.0.1 cast between incompatible types

[Thomas Monjalon]

09/05/2018 13:35, Reshma Pattan:
> Gcc 8.0.1 reports incompatible cast between types i.e. from
> `void (*)(void *)` to `(int (*)(void *)`.
> 
> Change the pipeline_stage prototype to retun int type
> to fix the issue.
> 
> Fixes: a0ffcb257a ("examples/quota_watermark: correct code indentation")
> CC: stable@dpdk.org
> 
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> ---
>  			/* typecast is a workaround for GCC 4.3 bug */
> -			rte_eal_remote_launch((int (*)(void *))pipeline_stage,
> +			rte_eal_remote_launch(pipeline_stage,
>  					NULL, lcore_id);

The comment can be removed.

Applied with above fixed, thanks

[PATCH] examples/vhost_scsi: replace strncpy with strlcpy

[Reshma Pattan]

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
-- 
2.14.3

Re: [PATCH] examples/vhost_scsi: replace strncpy with strlcpy

[Bruce Richardson]

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

Re: [PATCH] examples/vhost_scsi: replace strncpy with strlcpy

[Pattan, Reshma]

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

Re: [PATCH] examples/vhost_scsi: replace strncpy with strlcpy

[Pattan, Reshma]

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 v2] examples/vhost_scsi: replace strncpy with strlcpy

[Reshma Pattan]

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>
---
v2: replace magic numbers with sizeof() wherever possible.
---
 examples/vhost_scsi/scsi.c      | 14 +++++++++-----
 examples/vhost_scsi/scsi_spec.h |  2 +-
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/examples/vhost_scsi/scsi.c b/examples/vhost_scsi/scsi.c
index 2a034bb9b..0c2fa3e6a 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,8 @@ 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,
+					sizeof(vpage->params));
 			vpage->alloc_len = rte_cpu_to_be_16(32);
 			break;
 		case SPC_VPD_DEVICE_IDENTIFICATION:
@@ -215,10 +217,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 +277,8 @@ 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",
+			sizeof(inqdata->t10_vendor_id));
 
 		/* PRODUCT IDENTIFICATION */
 		snprintf((char *)inqdata->product_id,
@@ -283,7 +286,8 @@ 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",
+			sizeof(inqdata->product_rev));
 
 		/* Standard inquiry data ends here. Only populate
 		 * remaining fields if alloc_len indicates enough
diff --git a/examples/vhost_scsi/scsi_spec.h b/examples/vhost_scsi/scsi_spec.h
index 5c7a894bd..27be02688 100644
--- a/examples/vhost_scsi/scsi_spec.h
+++ b/examples/vhost_scsi/scsi_spec.h
@@ -367,7 +367,7 @@ struct scsi_vpd_page {
 	uint8_t peripheral;
 	uint8_t page_code;
 	uint16_t alloc_len;
-	uint8_t params[];
+	uint8_t params[32];
 };
 
 #define SCSI_VEXT_REF_CHK		0x01
-- 
2.17.0

Re: [PATCH v2] examples/vhost_scsi: replace strncpy with strlcpy

[Bruce Richardson]

On Thu, May 10, 2018 at 01:05:44PM +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>
> ---
> v2: replace magic numbers with sizeof() wherever possible.
> ---
>  examples/vhost_scsi/scsi.c      | 14 +++++++++-----
>  examples/vhost_scsi/scsi_spec.h |  2 +-
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
Looks better now, thanks.
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [PATCH v2] examples/vhost_scsi: replace strncpy with strlcpy

[Thomas Monjalon]

10/05/2018 15:31, Bruce Richardson:
> On Thu, May 10, 2018 at 01:05:44PM +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>
> > ---
> > v2: replace magic numbers with sizeof() wherever possible.
> > ---
> >  examples/vhost_scsi/scsi.c      | 14 +++++++++-----
> >  examples/vhost_scsi/scsi_spec.h |  2 +-
> >  2 files changed, 10 insertions(+), 6 deletions(-)
> > 
> Looks better now, thanks.
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Applied, thanks

Re: [PATCH] examples/ipsec-secgw: replace strncpy with strlcpy

[Bruce Richardson]

On Wed, May 09, 2018 at 12:35:27PM +0100, Reshma Pattan wrote:
> Use strlcpy instead of strncpy.
> 
> Fixes: 0d547ed037 ("examples/ipsec-secgw: support configuration file")
> Fixes: 07b156199f ("examples/ipsec-secgw: fix configuration string termination")
> Fixes: a1469c319f ("examples/ipsec-secgw: fix configuration parsing")
> Cc: stable@dpdk.org
> CC: Zhang,Roy Fan <roy.fan.zhang@intel.com>
> 
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> ---
>  examples/ipsec-secgw/parser.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/examples/ipsec-secgw/parser.c b/examples/ipsec-secgw/parser.c
> index 2403b564d..9ccd5ea72 100644
> --- a/examples/ipsec-secgw/parser.c
> +++ b/examples/ipsec-secgw/parser.c
> @@ -3,6 +3,7 @@
>   */
>  #include <rte_common.h>
>  #include <rte_crypto.h>
> +#include <rte_string_fns.h>
>  
>  #include <cmdline_parse_string.h>
>  #include <cmdline_parse_num.h>
> @@ -212,14 +213,14 @@ parse_ipv4_addr(const char *token, struct in_addr *ipv4, uint32_t *mask)
>  
>  	pch = strchr(token, '/');
>  	if (pch != NULL) {
> -		strncpy(ip_str, token, pch - token);
> +		strlcpy(ip_str, token, pch - token);

While this is fixing the compiler error, it's not really doing any bounds
checking for overflow on the destination buffer. Ideally, the final
parameter should be something like:
	min(pch - token, sizeof(ip_str))

>  		pch += 1;
>  		if (is_str_num(pch) != 0)
>  			return -EINVAL;
>  		if (mask)
>  			*mask = atoi(pch);
>  	} else {
> -		strncpy(ip_str, token, sizeof(ip_str) - 1);
> +		strlcpy(ip_str, token, sizeof(ip_str) - 1);

Since the original code was using strncpy, it's possible the "- 1" was an
incorrect attempt to make strncpy safe. Therefore, did you check to see if
it's possible to drop the -1 in the strlcpy case?

>  		if (mask)
>  			*mask = 0;
>  	}
> @@ -241,14 +242,14 @@ parse_ipv6_addr(const char *token, struct in6_addr *ipv6, uint32_t *mask)
>  
>  	pch = strchr(token, '/');
>  	if (pch != NULL) {
> -		strncpy(ip_str, token, pch - token);
> +		strlcpy(ip_str, token, pch - token);

As before, this doesn't do proper bounds checking.

>  		pch += 1;
>  		if (is_str_num(pch) != 0)
>  			return -EINVAL;
>  		if (mask)
>  			*mask = atoi(pch);
>  	} else {
> -		strncpy(ip_str, token, sizeof(ip_str) - 1);
> +		strlcpy(ip_str, token, sizeof(ip_str) - 1);

As before, can we remove the -1?

>  		if (mask)
>  			*mask = 0;
>  	}
> @@ -515,7 +516,7 @@ parse_cfg_file(const char *cfg_filename)
>  				goto error_exit;
>  			}
>  
> -			strncpy(str + strlen(str), oneline,
> +			strlcpy(str + strlen(str), oneline,
>  				strlen(oneline));

This doesn't do bounds checking, and since it just uses strlen to find the
bounds it can just be replaced by a strcpy() - which will also be more
efficient too, since it would only scan the string once, rather than twice
as here.

So, either add in a proper bounds check on the destination buffer, or if a
bounds check is not necessary, just replace with strcpy to show its not
actually needing a bounds check.

>  
>  			continue;
> @@ -528,7 +529,7 @@ parse_cfg_file(const char *cfg_filename)
>  				cfg_filename, line_num);
>  			goto error_exit;
>  		}
> -		strncpy(str + strlen(str), oneline,
> +		strlcpy(str + strlen(str), oneline,
>  			strlen(oneline));

As above.

>  
>  		str[strlen(str)] = '\n';
> -- 
> 2.14.3
> 

[PATCH v2] examples/ipsec-secgw: replace strncpy with strlcpy

[Reshma Pattan]

Use strlcpy instead of strncpy.
Use strcpy where boundchecks on destination is not needed.

Fixes: 0d547ed037 ("examples/ipsec-secgw: support configuration file")
Fixes: 07b156199f ("examples/ipsec-secgw: fix configuration string termination")
Fixes: a1469c319f ("examples/ipsec-secgw: fix configuration parsing")
Cc: stable@dpdk.org
CC: Zhang,Roy Fan <roy.fan.zhang@intel.com>

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
v2: use destination based boundary checks in strlcpy.
---
 examples/ipsec-secgw/parser.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/examples/ipsec-secgw/parser.c b/examples/ipsec-secgw/parser.c
index 2403b564d..2a669a6ad 100644
--- a/examples/ipsec-secgw/parser.c
+++ b/examples/ipsec-secgw/parser.c
@@ -3,6 +3,7 @@
  */
 #include <rte_common.h>
 #include <rte_crypto.h>
+#include <rte_string_fns.h>
 
 #include <cmdline_parse_string.h>
 #include <cmdline_parse_num.h>
@@ -207,19 +208,20 @@ inet_pton6(const char *src, unsigned char *dst)
 int
 parse_ipv4_addr(const char *token, struct in_addr *ipv4, uint32_t *mask)
 {
-	char ip_str[256] = {0};
+	char ip_str[INET_ADDRSTRLEN] = {0};
 	char *pch;
 
 	pch = strchr(token, '/');
 	if (pch != NULL) {
-		strncpy(ip_str, token, pch - token);
+		strlcpy(ip_str, token, RTE_MIN((long unsigned int)(pch - token),
+					sizeof(ip_str)));
 		pch += 1;
 		if (is_str_num(pch) != 0)
 			return -EINVAL;
 		if (mask)
 			*mask = atoi(pch);
 	} else {
-		strncpy(ip_str, token, sizeof(ip_str) - 1);
+		strlcpy(ip_str, token, sizeof(ip_str));
 		if (mask)
 			*mask = 0;
 	}
@@ -241,14 +243,15 @@ parse_ipv6_addr(const char *token, struct in6_addr *ipv6, uint32_t *mask)
 
 	pch = strchr(token, '/');
 	if (pch != NULL) {
-		strncpy(ip_str, token, pch - token);
+		strlcpy(ip_str, token, RTE_MIN((long unsigned int)(pch - token),
+					sizeof(ip_str)));
 		pch += 1;
 		if (is_str_num(pch) != 0)
 			return -EINVAL;
 		if (mask)
 			*mask = atoi(pch);
 	} else {
-		strncpy(ip_str, token, sizeof(ip_str) - 1);
+		strlcpy(ip_str, token, sizeof(ip_str));
 		if (mask)
 			*mask = 0;
 	}
@@ -515,9 +518,7 @@ parse_cfg_file(const char *cfg_filename)
 				goto error_exit;
 			}
 
-			strncpy(str + strlen(str), oneline,
-				strlen(oneline));
-
+			strcpy(str + strlen(str), oneline);
 			continue;
 		}
 
@@ -528,8 +529,7 @@ parse_cfg_file(const char *cfg_filename)
 				cfg_filename, line_num);
 			goto error_exit;
 		}
-		strncpy(str + strlen(str), oneline,
-			strlen(oneline));
+		strcpy(str + strlen(str), oneline);
 
 		str[strlen(str)] = '\n';
 		if (cmdline_parse(cl, str) < 0) {
-- 
2.17.0

[PATCH v3] examples/ipsec-secgw: replace strncpy with strlcpy

[Reshma Pattan]

Use strlcpy instead of strncpy.
Use strcpy where boundchecks on destination is not needed.

Fixes: 0d547ed037 ("examples/ipsec-secgw: support configuration file")
Fixes: 07b156199f ("examples/ipsec-secgw: fix configuration string termination")
Fixes: a1469c319f ("examples/ipsec-secgw: fix configuration parsing")
Cc: stable@dpdk.org
CC: Zhang,Roy Fan <roy.fan.zhang@intel.com>

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
v3: fix checkpatch warnings.
---
 examples/ipsec-secgw/parser.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/examples/ipsec-secgw/parser.c b/examples/ipsec-secgw/parser.c
index 2403b564d..e2e342908 100644
--- a/examples/ipsec-secgw/parser.c
+++ b/examples/ipsec-secgw/parser.c
@@ -3,6 +3,7 @@
  */
 #include <rte_common.h>
 #include <rte_crypto.h>
+#include <rte_string_fns.h>
 
 #include <cmdline_parse_string.h>
 #include <cmdline_parse_num.h>
@@ -207,19 +208,20 @@ inet_pton6(const char *src, unsigned char *dst)
 int
 parse_ipv4_addr(const char *token, struct in_addr *ipv4, uint32_t *mask)
 {
-	char ip_str[256] = {0};
+	char ip_str[INET_ADDRSTRLEN] = {0};
 	char *pch;
 
 	pch = strchr(token, '/');
 	if (pch != NULL) {
-		strncpy(ip_str, token, pch - token);
+		strlcpy(ip_str, token, RTE_MIN((unsigned int long)(pch - token),
+					sizeof(ip_str)));
 		pch += 1;
 		if (is_str_num(pch) != 0)
 			return -EINVAL;
 		if (mask)
 			*mask = atoi(pch);
 	} else {
-		strncpy(ip_str, token, sizeof(ip_str) - 1);
+		strlcpy(ip_str, token, sizeof(ip_str));
 		if (mask)
 			*mask = 0;
 	}
@@ -241,14 +243,15 @@ parse_ipv6_addr(const char *token, struct in6_addr *ipv6, uint32_t *mask)
 
 	pch = strchr(token, '/');
 	if (pch != NULL) {
-		strncpy(ip_str, token, pch - token);
+		strlcpy(ip_str, token, RTE_MIN((unsigned int long)(pch - token),
+					sizeof(ip_str)));
 		pch += 1;
 		if (is_str_num(pch) != 0)
 			return -EINVAL;
 		if (mask)
 			*mask = atoi(pch);
 	} else {
-		strncpy(ip_str, token, sizeof(ip_str) - 1);
+		strlcpy(ip_str, token, sizeof(ip_str));
 		if (mask)
 			*mask = 0;
 	}
@@ -515,9 +518,7 @@ parse_cfg_file(const char *cfg_filename)
 				goto error_exit;
 			}
 
-			strncpy(str + strlen(str), oneline,
-				strlen(oneline));
-
+			strcpy(str + strlen(str), oneline);
 			continue;
 		}
 
@@ -528,8 +529,7 @@ parse_cfg_file(const char *cfg_filename)
 				cfg_filename, line_num);
 			goto error_exit;
 		}
-		strncpy(str + strlen(str), oneline,
-			strlen(oneline));
+		strcpy(str + strlen(str), oneline);
 
 		str[strlen(str)] = '\n';
 		if (cmdline_parse(cl, str) < 0) {
-- 
2.17.0

Re: [dpdk-stable] [PATCH v3] examples/ipsec-secgw: replace strncpy with strlcpy

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: stable [mailto:stable-bounces@dpdk.org] On Behalf Of Reshma Pattan
> Sent: Wednesday, May 9, 2018 5:11 PM
> To: dev@dpdk.org
> Cc: Richardson, Bruce <bruce.richardson@intel.com>; stable@dpdk.org; Zhang,
> Roy Fan <roy.fan.zhang@intel.com>; Pattan, Reshma
> <reshma.pattan@intel.com>
> Subject: [dpdk-stable] [PATCH v3] examples/ipsec-secgw: replace strncpy with
> strlcpy
> 
> Use strlcpy instead of strncpy.
> Use strcpy where boundchecks on destination is not needed.
> 
> Fixes: 0d547ed037 ("examples/ipsec-secgw: support configuration file")
> Fixes: 07b156199f ("examples/ipsec-secgw: fix configuration string
> termination")
> Fixes: a1469c319f ("examples/ipsec-secgw: fix configuration parsing")
> Cc: stable@dpdk.org
> CC: Zhang,Roy Fan <roy.fan.zhang@intel.com>
> 
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>

Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

Re: [dpdk-stable] [PATCH v3] examples/ipsec-secgw: replace strncpy with strlcpy

[Thomas Monjalon]

> > Use strlcpy instead of strncpy.
> > Use strcpy where boundchecks on destination is not needed.
> > 
> > Fixes: 0d547ed037 ("examples/ipsec-secgw: support configuration file")
> > Fixes: 07b156199f ("examples/ipsec-secgw: fix configuration string
> > termination")
> > Fixes: a1469c319f ("examples/ipsec-secgw: fix configuration parsing")
> > Cc: stable@dpdk.org
> > CC: Zhang,Roy Fan <roy.fan.zhang@intel.com>
> > 
> > Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> 
> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

Applied, thanks