[dpdk-dev] [PATCH 0/3] fix coverity issues in packet capture framework

[dpdk-dev] [PATCH 0/3] fix coverity issues in packet capture framework

[Reshma Pattan]

This patchset fixes coverity issues in pdump library and pdump tool.

Reshma Pattan (3):
  pdump: check getenv return value
  pdump: fix string overflow
  app/pdump: fix string overflow

 app/pdump/main.c             |  4 ++--
 lib/librte_pdump/rte_pdump.c | 53 ++++++++++++++++++++++++++++++++++++--------
 2 files changed, 46 insertions(+), 11 deletions(-)

-- 
2.5.0

[dpdk-dev] [PATCH 1/3] pdump: check getenv return value

[Reshma Pattan]

getenv can return a NULL pointer if the match for
SOCKET_PATH_HOME is not found in the environment.
NULL check is added to return immediately without
calling mkdir.

Coverity issue 127344:  return value check
Coverity issue 127347:  null pointer dereference

Fixes: 278f945402c5 ("pdump: add new library for packet capture")
Fixes: 278f945402c5 ("pdump: add new library for packet capture")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 lib/librte_pdump/rte_pdump.c | 46 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 40 insertions(+), 6 deletions(-)

diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index c921f51..dbc6816 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -441,7 +441,7 @@ set_pdump_rxtx_cbs(struct pdump_request *p)
 }
 
 /* get socket path (/var/run if root, $HOME otherwise) */
-static void
+static int
 pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
 {
 	const char *dir = NULL;
@@ -451,8 +451,16 @@ pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
 	else if (type == RTE_PDUMP_SOCKET_CLIENT && client_socket_dir[0] != 0)
 		dir = client_socket_dir;
 	else {
-		if (getuid() != 0)
+		if (getuid() != 0) {
 			dir = getenv(SOCKET_PATH_HOME);
+			if (!dir) {
+				RTE_LOG(ERR, PDUMP,
+					"Failed to get environment variable"
+					"value for %s, %s:%d\n",
+					SOCKET_PATH_HOME, __func__, __LINE__);
+				return -1;
+			}
+		}
 		else
 			dir = SOCKET_PATH_VAR_RUN;
 	}
@@ -463,6 +471,8 @@ pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
 	else
 		snprintf(buffer, bufsz, CLIENT_SOCKET, dir, getpid(),
 				rte_sys_gettid());
+
+	return 0;
 }
 
 static int
@@ -472,8 +482,14 @@ pdump_create_server_socket(void)
 	struct sockaddr_un addr;
 	socklen_t addr_len;
 
-	pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
+	ret = pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
 				RTE_PDUMP_SOCKET_SERVER);
+	if (ret != 0) {
+		RTE_LOG(ERR, PDUMP,
+			"Failed to get server socket path: %s:%d\n",
+			__func__, __LINE__);
+		return -1;
+	}
 	addr.sun_family = AF_UNIX;
 
 	/* remove if file already exists */
@@ -604,8 +620,14 @@ rte_pdump_uninit(void)
 
 	struct sockaddr_un addr;
 
-	pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
+	ret = pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
 				RTE_PDUMP_SOCKET_SERVER);
+	if (ret != 0) {
+		RTE_LOG(ERR, PDUMP,
+			"Failed to get server socket path: %s:%d\n",
+			__func__, __LINE__);
+		return -1;
+	}
 	ret = unlink(addr.sun_path);
 	if (ret != 0) {
 		RTE_LOG(ERR, PDUMP,
@@ -639,8 +661,14 @@ pdump_create_client_socket(struct pdump_request *p)
 		return ret;
 	}
 
-	pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
+	ret = pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
 				RTE_PDUMP_SOCKET_CLIENT);
+	if (ret != 0) {
+		RTE_LOG(ERR, PDUMP,
+			"Failed to get client socket path: %s:%d\n",
+			__func__, __LINE__);
+		return -1;
+	}
 	addr.sun_family = AF_UNIX;
 	addr_len = sizeof(struct sockaddr_un);
 
@@ -656,9 +684,15 @@ pdump_create_client_socket(struct pdump_request *p)
 
 		serv_len = sizeof(struct sockaddr_un);
 		memset(&serv_addr, 0, sizeof(serv_addr));
-		pdump_get_socket_path(serv_addr.sun_path,
+		ret = pdump_get_socket_path(serv_addr.sun_path,
 					sizeof(serv_addr.sun_path),
 					RTE_PDUMP_SOCKET_SERVER);
+		if (ret != 0) {
+			RTE_LOG(ERR, PDUMP,
+			"Failed to get server socket path: %s:%d\n",
+			__func__, __LINE__);
+			break;
+		}
 		serv_addr.sun_family = AF_UNIX;
 
 		n =  sendto(socket_fd, p, sizeof(struct pdump_request), 0,
-- 
2.5.0

Re: [dpdk-dev] [PATCH 1/3] pdump: check getenv return value

[Ferruh Yigit]

On 6/21/2016 4:18 PM, Reshma Pattan wrote:
> getenv can return a NULL pointer if the match for
> SOCKET_PATH_HOME is not found in the environment.
> NULL check is added to return immediately without
> calling mkdir.
> 
> Coverity issue 127344:  return value check
> Coverity issue 127347:  null pointer dereference
> 
> Fixes: 278f945402c5 ("pdump: add new library for packet capture")
> Fixes: 278f945402c5 ("pdump: add new library for packet capture")
> 
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>

...

>  /* get socket path (/var/run if root, $HOME otherwise) */
> -static void
> +static int
>  pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
>  {
>  	const char *dir = NULL;
> @@ -451,8 +451,16 @@ pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
>  	else if (type == RTE_PDUMP_SOCKET_CLIENT && client_socket_dir[0] != 0)
>  		dir = client_socket_dir;
>  	else {
> -		if (getuid() != 0)
> +		if (getuid() != 0) {
>  			dir = getenv(SOCKET_PATH_HOME);
> +			if (!dir) {
> +				RTE_LOG(ERR, PDUMP,
> +					"Failed to get environment variable"
> +					"value for %s, %s:%d\n",
> +					SOCKET_PATH_HOME, __func__, __LINE__);
> +				return -1;
Instead of failing, does it make sense to fallback to a default path?
Is it possible that sometimes end user doesn't really care where socket
created as long as it created and runs smoothly?

Re: [dpdk-dev] [PATCH 1/3] pdump: check getenv return value

[Pattan, Reshma]

Hi Ferruh,

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Tuesday, June 21, 2016 5:56 PM
> To: Pattan, Reshma <reshma.pattan@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/3] pdump: check getenv return value
> 
> On 6/21/2016 4:18 PM, Reshma Pattan wrote:
> >  		dir = client_socket_dir;
> >  	else {
> > -		if (getuid() != 0)
> > +		if (getuid() != 0) {
> >  			dir = getenv(SOCKET_PATH_HOME);
> > +			if (!dir) {
> > +				RTE_LOG(ERR, PDUMP,
> > +					"Failed to get environment variable"
> > +					"value for %s, %s:%d\n",
> > +					SOCKET_PATH_HOME, __func__,
> __LINE__);
> > +				return -1;
> Instead of failing, does it make sense to fallback to a default path?
> Is it possible that sometimes end user doesn't really care where socket created
> as long as it created and runs smoothly?
> 
> 

This is for default path only. 

Thanks,
Reshma

[dpdk-dev] [PATCH 2/3] pdump: fix string overflow

[Reshma Pattan]

using source length in strncpy can cause destination
overflow if destination length is not big enough to
handle the source string. Changes are made to use destination
size instead of source length in strncpy.

Cverity issue 127350: string overflow

Fixes: 278f945402c5 ("pdump: add new library for packet capture")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 lib/librte_pdump/rte_pdump.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index dbc6816..05513d6 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -460,8 +460,7 @@ pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
 					SOCKET_PATH_HOME, __func__, __LINE__);
 				return -1;
 			}
-		}
-		else
+		} else
 			dir = SOCKET_PATH_VAR_RUN;
 	}
 
@@ -800,13 +799,15 @@ pdump_prepare_client_request(char *device, uint16_t queue,
 	req.flags = flags;
 	req.op =  operation;
 	if ((operation & ENABLE) != 0) {
-		strncpy(req.data.en_v1.device, device, strlen(device));
+		strncpy(req.data.en_v1.device, device,
+			sizeof(req.data.en_v1.device)-1);
 		req.data.en_v1.queue = queue;
 		req.data.en_v1.ring = ring;
 		req.data.en_v1.mp = mp;
 		req.data.en_v1.filter = filter;
 	} else {
-		strncpy(req.data.dis_v1.device, device, strlen(device));
+		strncpy(req.data.dis_v1.device, device,
+			sizeof(req.data.dis_v1.device)-1);
 		req.data.dis_v1.queue = queue;
 		req.data.dis_v1.ring = NULL;
 		req.data.dis_v1.mp = NULL;
-- 
2.5.0

Re: [dpdk-dev] [PATCH 2/3] pdump: fix string overflow

[Ferruh Yigit]

On 6/21/2016 4:18 PM, Reshma Pattan wrote:
> using source length in strncpy can cause destination
> overflow if destination length is not big enough to
> handle the source string. Changes are made to use destination
> size instead of source length in strncpy.
> 
> Cverity issue 127350: string overflow
> 
> Fixes: 278f945402c5 ("pdump: add new library for packet capture")
> 
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> ---
>  lib/librte_pdump/rte_pdump.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
> index dbc6816..05513d6 100644
> --- a/lib/librte_pdump/rte_pdump.c
> +++ b/lib/librte_pdump/rte_pdump.c
> @@ -460,8 +460,7 @@ pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
>  					SOCKET_PATH_HOME, __func__, __LINE__);
>  				return -1;
>  			}
> -		}
> -		else
> +		} else
syntax fix may be not belong to this patch

>  			dir = SOCKET_PATH_VAR_RUN;
>  	}
>  
> @@ -800,13 +799,15 @@ pdump_prepare_client_request(char *device, uint16_t queue,
>  	req.flags = flags;
>  	req.op =  operation;
>  	if ((operation & ENABLE) != 0) {
> -		strncpy(req.data.en_v1.device, device, strlen(device));
> +		strncpy(req.data.en_v1.device, device,
> +			sizeof(req.data.en_v1.device)-1);
"-" missing spaces around

[dpdk-dev] [PATCH 3/3] app/pdump: fix string overflow

[Reshma Pattan]

using source length in strncpy can cause destination
overflow if destination length is not big enough to
handle the source string. Changes are made to use destination
size instead of source length in strncpy.

Coverity issue 127351: string overflow

Fixes: caa7028276b8 ("app/pdump: add tool for packet capturing")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 app/pdump/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/pdump/main.c b/app/pdump/main.c
index f8923b9..af92ef3 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -217,12 +217,12 @@ parse_rxtxdev(const char *key, const char *value, void *extra_args)
 	struct pdump_tuples *pt = extra_args;
 
 	if (!strcmp(key, PDUMP_RX_DEV_ARG)) {
-		strncpy(pt->rx_dev, value, strlen(value));
+		strncpy(pt->rx_dev, value, sizeof(pt->rx_dev)-1);
 		/* identify the tx stream type for pcap vdev */
 		if (if_nametoindex(pt->rx_dev))
 			pt->rx_vdev_stream_type = IFACE;
 	} else if (!strcmp(key, PDUMP_TX_DEV_ARG)) {
-		strncpy(pt->tx_dev, value, strlen(value));
+		strncpy(pt->tx_dev, value, sizeof(pt->tx_dev)-1);
 		/* identify the tx stream type for pcap vdev */
 		if (if_nametoindex(pt->tx_dev))
 			pt->tx_vdev_stream_type = IFACE;
-- 
2.5.0

Re: [dpdk-dev] [PATCH 3/3] app/pdump: fix string overflow

[Ferruh Yigit]

On 6/21/2016 4:18 PM, Reshma Pattan wrote:
> using source length in strncpy can cause destination
> overflow if destination length is not big enough to
> handle the source string. Changes are made to use destination
> size instead of source length in strncpy.
> 
> Coverity issue 127351: string overflow
> 
> Fixes: caa7028276b8 ("app/pdump: add tool for packet capturing")
> 
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> ---
>  app/pdump/main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/app/pdump/main.c b/app/pdump/main.c
> index f8923b9..af92ef3 100644
> --- a/app/pdump/main.c
> +++ b/app/pdump/main.c
> @@ -217,12 +217,12 @@ parse_rxtxdev(const char *key, const char *value, void *extra_args)
>  	struct pdump_tuples *pt = extra_args;
>  
>  	if (!strcmp(key, PDUMP_RX_DEV_ARG)) {
> -		strncpy(pt->rx_dev, value, strlen(value));
> +		strncpy(pt->rx_dev, value, sizeof(pt->rx_dev)-1);

I guess size-1 is to give room for terminating null byte, but for this
case is it guarantied that pt->rx_dev last byte is NULL?

Re: [dpdk-dev] [PATCH 3/3] app/pdump: fix string overflow

[Anupam Kapoor]

>       if (!strcmp(key, PDUMP_RX_DEV_ARG)) {
> -             strncpy(pt->rx_dev, value, strlen(value));
> +             strncpy(pt->rx_dev, value, sizeof(pt->rx_dev)-1);

I guess size-1 is to give room for terminating null byte, but for this
case is it guarantied that pt->rx_dev last byte is NULL?

why not just use a snprintf(...) here since it has better error behavior ?
although compared to str*cpy it might be a bit slow, but hopefully that
should be ok ?

--
thanks
anupam


On Tue, Jun 21, 2016 at 10:51 PM, Ferruh Yigit <ferruh.yigit@intel.com>
wrote:

> On 6/21/2016 4:18 PM, Reshma Pattan wrote:
> > using source length in strncpy can cause destination
> > overflow if destination length is not big enough to
> > handle the source string. Changes are made to use destination
> > size instead of source length in strncpy.
> >
> > Coverity issue 127351: string overflow
> >
> > Fixes: caa7028276b8 ("app/pdump: add tool for packet capturing")
> >
> > Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> > ---
> >  app/pdump/main.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/app/pdump/main.c b/app/pdump/main.c
> > index f8923b9..af92ef3 100644
> > --- a/app/pdump/main.c
> > +++ b/app/pdump/main.c
> > @@ -217,12 +217,12 @@ parse_rxtxdev(const char *key, const char *value,
> void *extra_args)
> >       struct pdump_tuples *pt = extra_args;
> >
> >       if (!strcmp(key, PDUMP_RX_DEV_ARG)) {
> > -             strncpy(pt->rx_dev, value, strlen(value));
> > +             strncpy(pt->rx_dev, value, sizeof(pt->rx_dev)-1);
>
> I guess size-1 is to give room for terminating null byte, but for this
> case is it guarantied that pt->rx_dev last byte is NULL?
>
>


-- 
In the beginning was the lambda, and the lambda was with Emacs, and Emacs
was the lambda.

Re: [dpdk-dev] [PATCH 3/3] app/pdump: fix string overflow

[Bruce Richardson]

On Wed, Jun 22, 2016 at 12:16:27PM +0530, Anupam Kapoor wrote:
> >       if (!strcmp(key, PDUMP_RX_DEV_ARG)) {
> > -             strncpy(pt->rx_dev, value, strlen(value));
> > +             strncpy(pt->rx_dev, value, sizeof(pt->rx_dev)-1);
> 
> I guess size-1 is to give room for terminating null byte, but for this
> case is it guarantied that pt->rx_dev last byte is NULL?
> 
> why not just use a snprintf(...) here since it has better error behavior ?
> although compared to str*cpy it might be a bit slow, but hopefully that
> should be ok ?
> 

Definite +1. For safely copying strings I think snprintf is often the easiest
API to use.

/Bruce

> --
> thanks
> anupam
> 
> 
> On Tue, Jun 21, 2016 at 10:51 PM, Ferruh Yigit <ferruh.yigit@intel.com>
> wrote:
> 
> > On 6/21/2016 4:18 PM, Reshma Pattan wrote:
> > > using source length in strncpy can cause destination
> > > overflow if destination length is not big enough to
> > > handle the source string. Changes are made to use destination
> > > size instead of source length in strncpy.
> > >
> > > Coverity issue 127351: string overflow
> > >
> > > Fixes: caa7028276b8 ("app/pdump: add tool for packet capturing")
> > >
> > > Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> > > ---
> > >  app/pdump/main.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/app/pdump/main.c b/app/pdump/main.c
> > > index f8923b9..af92ef3 100644
> > > --- a/app/pdump/main.c
> > > +++ b/app/pdump/main.c
> > > @@ -217,12 +217,12 @@ parse_rxtxdev(const char *key, const char *value,
> > void *extra_args)
> > >       struct pdump_tuples *pt = extra_args;
> > >
> > >       if (!strcmp(key, PDUMP_RX_DEV_ARG)) {
> > > -             strncpy(pt->rx_dev, value, strlen(value));
> > > +             strncpy(pt->rx_dev, value, sizeof(pt->rx_dev)-1);
> >
> > I guess size-1 is to give room for terminating null byte, but for this
> > case is it guarantied that pt->rx_dev last byte is NULL?
> >
> >
> 
> 
> -- 
> In the beginning was the lambda, and the lambda was with Emacs, and Emacs
> was the lambda.

Re: [dpdk-dev] [PATCH 3/3] app/pdump: fix string overflow

[Pattan, Reshma]

Hi,

> -----Original Message-----
> From: Richardson, Bruce
> Sent: Wednesday, June 22, 2016 10:22 AM
> To: Anupam Kapoor <anupam.kapoor@gmail.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Pattan, Reshma
> <reshma.pattan@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 3/3] app/pdump: fix string overflow
> 
> On Wed, Jun 22, 2016 at 12:16:27PM +0530, Anupam Kapoor wrote:
> > >       if (!strcmp(key, PDUMP_RX_DEV_ARG)) {
> > > -             strncpy(pt->rx_dev, value, strlen(value));
> > > +             strncpy(pt->rx_dev, value, sizeof(pt->rx_dev)-1);
> >
> > I guess size-1 is to give room for terminating null byte, but for this
> > case is it guarantied that pt->rx_dev last byte is NULL?
> >
> > why not just use a snprintf(...) here since it has better error behavior ?
> > although compared to str*cpy it might be a bit slow, but hopefully
> > that should be ok ?
> >
> 
> Definite +1. For safely copying strings I think snprintf is often the easiest API to
> use.
> 

Ok, will make the changes.

Thanks,
Reshma

[dpdk-dev] [PATCH v2 0/3] fix coverity issues in packet capture framework

[Reshma Pattan]

This patchset fixes coverity issues in pdump library and pdump tool.

v2:
fixed code review comment to use snprintf instead of strncpy.

Reshma Pattan (3):
  pdump: check getenv return value
  pdump: fix string overflow
  app/pdump: fix string overflow

 app/pdump/main.c             |  4 ++--
 lib/librte_pdump/rte_pdump.c | 53 ++++++++++++++++++++++++++++++++++++--------
 2 files changed, 46 insertions(+), 11 deletions(-)

-- 
2.5.0

[dpdk-dev] [PATCH v2 1/3] pdump: check getenv return value

[Reshma Pattan]

inside pdump_get_socket_path(), getenv can return
a NULL pointer if the match for SOCKET_PATH_HOME is
not found in the environment. NULL check is added to
return -1 immediately without calling mkdir.
Since pdump_get_socket_path() returns -1 now,
wherever this function is called there the return
value is checked and error message is logged.

Coverity issue 127344:  return value check
Coverity issue 127347:  null pointer dereference

Fixes: 278f945402c5 ("pdump: add new library for packet capture")
Fixes: 278f945402c5 ("pdump: add new library for packet capture")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 lib/librte_pdump/rte_pdump.c | 47 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 40 insertions(+), 7 deletions(-)

diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index c921f51..de20b9e 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -441,7 +441,7 @@ set_pdump_rxtx_cbs(struct pdump_request *p)
 }
 
 /* get socket path (/var/run if root, $HOME otherwise) */
-static void
+static int
 pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
 {
 	const char *dir = NULL;
@@ -451,9 +451,16 @@ pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
 	else if (type == RTE_PDUMP_SOCKET_CLIENT && client_socket_dir[0] != 0)
 		dir = client_socket_dir;
 	else {
-		if (getuid() != 0)
+		if (getuid() != 0) {
 			dir = getenv(SOCKET_PATH_HOME);
-		else
+			if (!dir) {
+				RTE_LOG(ERR, PDUMP,
+					"Failed to get environment variable"
+					" value for %s, %s:%d\n",
+					SOCKET_PATH_HOME, __func__, __LINE__);
+				return -1;
+			}
+		} else
 			dir = SOCKET_PATH_VAR_RUN;
 	}
 
@@ -463,6 +470,8 @@ pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
 	else
 		snprintf(buffer, bufsz, CLIENT_SOCKET, dir, getpid(),
 				rte_sys_gettid());
+
+	return 0;
 }
 
 static int
@@ -472,8 +481,14 @@ pdump_create_server_socket(void)
 	struct sockaddr_un addr;
 	socklen_t addr_len;
 
-	pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
+	ret = pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
 				RTE_PDUMP_SOCKET_SERVER);
+	if (ret != 0) {
+		RTE_LOG(ERR, PDUMP,
+			"Failed to get server socket path: %s:%d\n",
+			__func__, __LINE__);
+		return -1;
+	}
 	addr.sun_family = AF_UNIX;
 
 	/* remove if file already exists */
@@ -604,8 +619,14 @@ rte_pdump_uninit(void)
 
 	struct sockaddr_un addr;
 
-	pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
+	ret = pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
 				RTE_PDUMP_SOCKET_SERVER);
+	if (ret != 0) {
+		RTE_LOG(ERR, PDUMP,
+			"Failed to get server socket path: %s:%d\n",
+			__func__, __LINE__);
+		return -1;
+	}
 	ret = unlink(addr.sun_path);
 	if (ret != 0) {
 		RTE_LOG(ERR, PDUMP,
@@ -639,8 +660,14 @@ pdump_create_client_socket(struct pdump_request *p)
 		return ret;
 	}
 
-	pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
+	ret = pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
 				RTE_PDUMP_SOCKET_CLIENT);
+	if (ret != 0) {
+		RTE_LOG(ERR, PDUMP,
+			"Failed to get client socket path: %s:%d\n",
+			__func__, __LINE__);
+		return -1;
+	}
 	addr.sun_family = AF_UNIX;
 	addr_len = sizeof(struct sockaddr_un);
 
@@ -656,9 +683,15 @@ pdump_create_client_socket(struct pdump_request *p)
 
 		serv_len = sizeof(struct sockaddr_un);
 		memset(&serv_addr, 0, sizeof(serv_addr));
-		pdump_get_socket_path(serv_addr.sun_path,
+		ret = pdump_get_socket_path(serv_addr.sun_path,
 					sizeof(serv_addr.sun_path),
 					RTE_PDUMP_SOCKET_SERVER);
+		if (ret != 0) {
+			RTE_LOG(ERR, PDUMP,
+			"Failed to get server socket path: %s:%d\n",
+			__func__, __LINE__);
+			break;
+		}
 		serv_addr.sun_family = AF_UNIX;
 
 		n =  sendto(socket_fd, p, sizeof(struct pdump_request), 0,
-- 
2.5.0

[dpdk-dev] [PATCH v2 2/3] pdump: fix string overflow

[Reshma Pattan]

replaced strncpy with snprintf for safely
copying the strings.

Cverity issue 127350: string overflow

Fixes: 278f945402c5 ("pdump: add new library for packet capture")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 lib/librte_pdump/rte_pdump.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index de20b9e..b499d09 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -799,13 +799,15 @@ pdump_prepare_client_request(char *device, uint16_t queue,
 	req.flags = flags;
 	req.op =  operation;
 	if ((operation & ENABLE) != 0) {
-		strncpy(req.data.en_v1.device, device, strlen(device));
+		snprintf(req.data.en_v1.device, sizeof(req.data.en_v1.device),
+				"%s", device);
 		req.data.en_v1.queue = queue;
 		req.data.en_v1.ring = ring;
 		req.data.en_v1.mp = mp;
 		req.data.en_v1.filter = filter;
 	} else {
-		strncpy(req.data.dis_v1.device, device, strlen(device));
+		snprintf(req.data.dis_v1.device, sizeof(req.data.dis_v1.device),
+				"%s", device);
 		req.data.dis_v1.queue = queue;
 		req.data.dis_v1.ring = NULL;
 		req.data.dis_v1.mp = NULL;
-- 
2.5.0

[dpdk-dev] [PATCH v2 3/3] app/pdump: fix string overflow

[Reshma Pattan]

replaced strncpy with snprintf for safely
copying the strings.

Coverity issue 127351: string overflow

Fixes: caa7028276b8 ("app/pdump: add tool for packet capturing")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 app/pdump/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/pdump/main.c b/app/pdump/main.c
index f8923b9..fe4d38a 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -217,12 +217,12 @@ parse_rxtxdev(const char *key, const char *value, void *extra_args)
 	struct pdump_tuples *pt = extra_args;
 
 	if (!strcmp(key, PDUMP_RX_DEV_ARG)) {
-		strncpy(pt->rx_dev, value, strlen(value));
+		snprintf(pt->rx_dev, sizeof(pt->rx_dev), "%s", value);
 		/* identify the tx stream type for pcap vdev */
 		if (if_nametoindex(pt->rx_dev))
 			pt->rx_vdev_stream_type = IFACE;
 	} else if (!strcmp(key, PDUMP_TX_DEV_ARG)) {
-		strncpy(pt->tx_dev, value, strlen(value));
+		snprintf(pt->tx_dev, sizeof(pt->tx_dev), "%s", value);
 		/* identify the tx stream type for pcap vdev */
 		if (if_nametoindex(pt->tx_dev))
 			pt->tx_vdev_stream_type = IFACE;
-- 
2.5.0

[dpdk-dev] [PATCH v3 0/4] fix issues in packet capture framework

[Reshma Pattan]

This patchset includes listed fixes
1)fix default socket path in pdump library.
2)fix coverity issues in pdump library.
3)fix coverity issues in pdump tool.

v3:
added new patch for fixing default socket paths "HOME" and "/var/run".
reworked coverity fixes on top of the above change.

v2:
fixed code review comment to use snprintf instead of strncpy.

Reshma Pattan (4):
  pdump: fix default socket path
  pdump: check getenv return value
  pdump: fix string overflow
  app/pdump: fix string overflow

 app/pdump/main.c             |  4 +--
 lib/librte_pdump/rte_pdump.c | 72 ++++++++++++++++++++++++++++++++++----------
 2 files changed, 58 insertions(+), 18 deletions(-)

-- 
2.5.0

[dpdk-dev] [PATCH v3 1/4] pdump: fix default socket path

[Reshma Pattan]

SOCKET_PATH_HOME is to specify environment variable "HOME",
so it should not contain "/pdump_sockets"  in the macro.
So remove "/pdump_sockets" from SOCKET_PATH_HOME
and create new macro for "/pdump_sockets". Similary removed
"/pdump_sockets" from SOCKET_PATH_VAR_RUN.
Changes are done in pdump_get_socket_path() to accommodate
new socket path changes.

Fixes: 278f945402c5 ("pdump: add new library for packet capture")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 lib/librte_pdump/rte_pdump.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index c921f51..5c335ba 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -50,8 +50,9 @@
 
 #include "rte_pdump.h"
 
-#define SOCKET_PATH_VAR_RUN "/var/run/pdump_sockets"
-#define SOCKET_PATH_HOME "HOME/pdump_sockets"
+#define SOCKET_PATH_VAR_RUN "/var/run"
+#define SOCKET_PATH_HOME "HOME"
+#define SOCKET_DIR       "/pdump_sockets"
 #define SERVER_SOCKET "%s/pdump_server_socket"
 #define CLIENT_SOCKET "%s/pdump_client_socket_%d_%u"
 #define DEVICE_ID_SIZE 64
@@ -444,17 +445,20 @@ set_pdump_rxtx_cbs(struct pdump_request *p)
 static void
 pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
 {
-	const char *dir = NULL;
+	char dir[PATH_MAX] = {0};
+	char *dir_home = NULL;
 
 	if (type == RTE_PDUMP_SOCKET_SERVER && server_socket_dir[0] != 0)
-		dir = server_socket_dir;
+		snprintf(dir, sizeof(dir), "%s", server_socket_dir);
 	else if (type == RTE_PDUMP_SOCKET_CLIENT && client_socket_dir[0] != 0)
-		dir = client_socket_dir;
+		snprintf(dir, sizeof(dir), "%s", client_socket_dir);
 	else {
-		if (getuid() != 0)
-			dir = getenv(SOCKET_PATH_HOME);
-		else
-			dir = SOCKET_PATH_VAR_RUN;
+		if (getuid() != 0) {
+			dir_home = getenv(SOCKET_PATH_HOME);
+			strcat(dir, dir_home);
+		} else
+			strcat(dir, SOCKET_PATH_VAR_RUN);
+		strcat(dir, SOCKET_DIR);
 	}
 
 	mkdir(dir, 700);
-- 
2.5.0

[dpdk-dev] [PATCH v3 2/4] pdump: check getenv return value

[Reshma Pattan]

inside pdump_get_socket_path(), getenv can return
a NULL pointer if the match for SOCKET_PATH_HOME is
not found in the environment. NULL check is added to
return -1 immediately. Since pdump_get_socket_path()
returns -1 now, wherever this function is called
there the return value is checked and error message
is logged.

Coverity issue 127344:  return value check
Coverity issue 127347:  null pointer dereference

Fixes: 278f945402c5 ("pdump: add new library for packet capture")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 lib/librte_pdump/rte_pdump.c | 44 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index 5c335ba..8240387 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -442,7 +442,7 @@ set_pdump_rxtx_cbs(struct pdump_request *p)
 }
 
 /* get socket path (/var/run if root, $HOME otherwise) */
-static void
+static int
 pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
 {
 	char dir[PATH_MAX] = {0};
@@ -455,9 +455,17 @@ pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
 	else {
 		if (getuid() != 0) {
 			dir_home = getenv(SOCKET_PATH_HOME);
+			if (!dir_home) {
+				RTE_LOG(ERR, PDUMP,
+					"Failed to get environment variable"
+					" value for %s, %s:%d\n",
+					SOCKET_PATH_HOME, __func__, __LINE__);
+				return -1;
+			}
 			strcat(dir, dir_home);
 		} else
 			strcat(dir, SOCKET_PATH_VAR_RUN);
+
 		strcat(dir, SOCKET_DIR);
 	}
 
@@ -467,6 +475,8 @@ pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
 	else
 		snprintf(buffer, bufsz, CLIENT_SOCKET, dir, getpid(),
 				rte_sys_gettid());
+
+	return 0;
 }
 
 static int
@@ -476,8 +486,14 @@ pdump_create_server_socket(void)
 	struct sockaddr_un addr;
 	socklen_t addr_len;
 
-	pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
+	ret = pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
 				RTE_PDUMP_SOCKET_SERVER);
+	if (ret != 0) {
+		RTE_LOG(ERR, PDUMP,
+			"Failed to get server socket path: %s:%d\n",
+			__func__, __LINE__);
+		return -1;
+	}
 	addr.sun_family = AF_UNIX;
 
 	/* remove if file already exists */
@@ -608,8 +624,14 @@ rte_pdump_uninit(void)
 
 	struct sockaddr_un addr;
 
-	pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
+	ret = pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
 				RTE_PDUMP_SOCKET_SERVER);
+	if (ret != 0) {
+		RTE_LOG(ERR, PDUMP,
+			"Failed to get server socket path: %s:%d\n",
+			__func__, __LINE__);
+		return -1;
+	}
 	ret = unlink(addr.sun_path);
 	if (ret != 0) {
 		RTE_LOG(ERR, PDUMP,
@@ -643,8 +665,14 @@ pdump_create_client_socket(struct pdump_request *p)
 		return ret;
 	}
 
-	pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
+	ret = pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
 				RTE_PDUMP_SOCKET_CLIENT);
+	if (ret != 0) {
+		RTE_LOG(ERR, PDUMP,
+			"Failed to get client socket path: %s:%d\n",
+			__func__, __LINE__);
+		return -1;
+	}
 	addr.sun_family = AF_UNIX;
 	addr_len = sizeof(struct sockaddr_un);
 
@@ -660,9 +688,15 @@ pdump_create_client_socket(struct pdump_request *p)
 
 		serv_len = sizeof(struct sockaddr_un);
 		memset(&serv_addr, 0, sizeof(serv_addr));
-		pdump_get_socket_path(serv_addr.sun_path,
+		ret = pdump_get_socket_path(serv_addr.sun_path,
 					sizeof(serv_addr.sun_path),
 					RTE_PDUMP_SOCKET_SERVER);
+		if (ret != 0) {
+			RTE_LOG(ERR, PDUMP,
+				"Failed to get server socket path: %s:%d\n",
+				__func__, __LINE__);
+			break;
+		}
 		serv_addr.sun_family = AF_UNIX;
 
 		n =  sendto(socket_fd, p, sizeof(struct pdump_request), 0,
-- 
2.5.0

[dpdk-dev] [PATCH v3 3/4] pdump: fix string overflow

[Reshma Pattan]

replaced strncpy with snprintf for safely
copying the strings.

Cverity issue 127350: string overflow

Fixes: 278f945402c5 ("pdump: add new library for packet capture")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 lib/librte_pdump/rte_pdump.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index 8240387..53a5bf2 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -804,13 +804,15 @@ pdump_prepare_client_request(char *device, uint16_t queue,
 	req.flags = flags;
 	req.op =  operation;
 	if ((operation & ENABLE) != 0) {
-		strncpy(req.data.en_v1.device, device, strlen(device));
+		snprintf(req.data.en_v1.device, sizeof(req.data.en_v1.device),
+				"%s", device);
 		req.data.en_v1.queue = queue;
 		req.data.en_v1.ring = ring;
 		req.data.en_v1.mp = mp;
 		req.data.en_v1.filter = filter;
 	} else {
-		strncpy(req.data.dis_v1.device, device, strlen(device));
+		snprintf(req.data.dis_v1.device, sizeof(req.data.dis_v1.device),
+				"%s", device);
 		req.data.dis_v1.queue = queue;
 		req.data.dis_v1.ring = NULL;
 		req.data.dis_v1.mp = NULL;
-- 
2.5.0

[dpdk-dev] [PATCH v3 4/4] app/pdump: fix string overflow

[Reshma Pattan]

replaced strncpy with snprintf for safely
copying the strings.

Coverity issue 127351: string overflow

Fixes: caa7028276b8 ("app/pdump: add tool for packet capturing")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 app/pdump/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/pdump/main.c b/app/pdump/main.c
index f8923b9..fe4d38a 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -217,12 +217,12 @@ parse_rxtxdev(const char *key, const char *value, void *extra_args)
 	struct pdump_tuples *pt = extra_args;
 
 	if (!strcmp(key, PDUMP_RX_DEV_ARG)) {
-		strncpy(pt->rx_dev, value, strlen(value));
+		snprintf(pt->rx_dev, sizeof(pt->rx_dev), "%s", value);
 		/* identify the tx stream type for pcap vdev */
 		if (if_nametoindex(pt->rx_dev))
 			pt->rx_vdev_stream_type = IFACE;
 	} else if (!strcmp(key, PDUMP_TX_DEV_ARG)) {
-		strncpy(pt->tx_dev, value, strlen(value));
+		snprintf(pt->tx_dev, sizeof(pt->tx_dev), "%s", value);
 		/* identify the tx stream type for pcap vdev */
 		if (if_nametoindex(pt->tx_dev))
 			pt->tx_vdev_stream_type = IFACE;
-- 
2.5.0

[dpdk-dev] [PATCH v4 0/5] fix issues in packet capture framework

[Reshma Pattan]

This patchset includes listed fixes
1)fix default socket path in pdump library.
2)fix coverity issues in pdump library.
3)fix coverity issues in pdump tool.
4)fix wrong typecast of ring size in pdump tool.

v4:
added new patch for fixing wrong typecast of ring size
in pdump tool.

v3:
added new patch for fixing default socket paths "HOME" and "/var/run".
reworked coverity fixes on top of the above change.

v2:
fixed code review comment to use snprintf instead of strncpy.

Reshma Pattan (5):
  pdump: fix default socket path
  pdump: check getenv return value
  pdump: fix string overflow
  app/pdump: fix string overflow
  app/pdump: fix type casting of ring size

 app/pdump/main.c             |  6 ++--
 lib/librte_pdump/rte_pdump.c | 72 ++++++++++++++++++++++++++++++++++----------
 2 files changed, 59 insertions(+), 19 deletions(-)

-- 
2.5.0

[dpdk-dev] [PATCH v4 1/5] pdump: fix default socket path

[Reshma Pattan]

SOCKET_PATH_HOME is to specify environment variable "HOME",
so it should not contain "/pdump_sockets"  in the macro.
So remove "/pdump_sockets" from SOCKET_PATH_HOME
and create new macro for "/pdump_sockets". Similary removed
"/pdump_sockets" from SOCKET_PATH_VAR_RUN.
Changes are done in pdump_get_socket_path() to accommodate
new socket path changes.

Fixes: 278f945402c5 ("pdump: add new library for packet capture")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 lib/librte_pdump/rte_pdump.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index c921f51..5c335ba 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -50,8 +50,9 @@
 
 #include "rte_pdump.h"
 
-#define SOCKET_PATH_VAR_RUN "/var/run/pdump_sockets"
-#define SOCKET_PATH_HOME "HOME/pdump_sockets"
+#define SOCKET_PATH_VAR_RUN "/var/run"
+#define SOCKET_PATH_HOME "HOME"
+#define SOCKET_DIR       "/pdump_sockets"
 #define SERVER_SOCKET "%s/pdump_server_socket"
 #define CLIENT_SOCKET "%s/pdump_client_socket_%d_%u"
 #define DEVICE_ID_SIZE 64
@@ -444,17 +445,20 @@ set_pdump_rxtx_cbs(struct pdump_request *p)
 static void
 pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
 {
-	const char *dir = NULL;
+	char dir[PATH_MAX] = {0};
+	char *dir_home = NULL;
 
 	if (type == RTE_PDUMP_SOCKET_SERVER && server_socket_dir[0] != 0)
-		dir = server_socket_dir;
+		snprintf(dir, sizeof(dir), "%s", server_socket_dir);
 	else if (type == RTE_PDUMP_SOCKET_CLIENT && client_socket_dir[0] != 0)
-		dir = client_socket_dir;
+		snprintf(dir, sizeof(dir), "%s", client_socket_dir);
 	else {
-		if (getuid() != 0)
-			dir = getenv(SOCKET_PATH_HOME);
-		else
-			dir = SOCKET_PATH_VAR_RUN;
+		if (getuid() != 0) {
+			dir_home = getenv(SOCKET_PATH_HOME);
+			strcat(dir, dir_home);
+		} else
+			strcat(dir, SOCKET_PATH_VAR_RUN);
+		strcat(dir, SOCKET_DIR);
 	}
 
 	mkdir(dir, 700);
-- 
2.5.0

Re: [dpdk-dev] [PATCH v4 1/5] pdump: fix default socket path

[Thomas Monjalon]

2016-06-24 14:54, Reshma Pattan:
> +#define SOCKET_DIR       "/pdump_sockets"

I think the default socket directory should contain dpdk as prefix.
Like dpdk-pdump-sockets (I think dash is preferred for filenames).
I wonder whether it should be a hidden directory:
	~/.dpdk-pdump-sockets
And after all, why not simply
	~/.dpdk/
It would allow other DPDK applications to put some files.

Re: [dpdk-dev] [PATCH v4 1/5] pdump: fix default socket path

[Pattan, Reshma]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Friday, June 24, 2016 3:55 PM
> To: Pattan, Reshma <reshma.pattan@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 1/5] pdump: fix default socket path
> 
> 2016-06-24 14:54, Reshma Pattan:
> > +#define SOCKET_DIR       "/pdump_sockets"
> 
> I think the default socket directory should contain dpdk as prefix.
> Like dpdk-pdump-sockets (I think dash is preferred for filenames).
> I wonder whether it should be a hidden directory:
> 	~/.dpdk-pdump-sockets
> And after all, why not simply
> 	~/.dpdk/

Hmm I would keep the name dpdk-pdump-sockets as library creates this only for pdump sockets.
Hidden directory just gives the advantage of not cluttering the display. If your comment is for the same I can add a . in the directory name:-).

> It would allow other DPDK applications to put some files.

Re: [dpdk-dev] [PATCH v4 1/5] pdump: fix default socket path

[Pattan, Reshma]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Friday, June 24, 2016 3:55 PM
> To: Pattan, Reshma <reshma.pattan@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 1/5] pdump: fix default socket path
> 
> 2016-06-24 14:54, Reshma Pattan:
> > +#define SOCKET_DIR       "/pdump_sockets"
> 
> I think the default socket directory should contain dpdk as prefix.
> Like dpdk-pdump-sockets (I think dash is preferred for filenames).
> I wonder whether it should be a hidden directory:
> 	~/.dpdk-pdump-sockets
> And after all, why not simply
> 	~/.dpdk/

patch v5 is sent with changes /.dpdk/pdump_sockets.

> It would allow other DPDK applications to put some files.

[dpdk-dev] [PATCH v4 2/5] pdump: check getenv return value

[Reshma Pattan]

inside pdump_get_socket_path(), getenv can return
a NULL pointer if the match for SOCKET_PATH_HOME is
not found in the environment. NULL check is added to
return -1 immediately. Since pdump_get_socket_path()
returns -1 now, wherever this function is called
there the return value is checked and error message
is logged.

Coverity issue 127344:  return value check
Coverity issue 127347:  null pointer dereference

Fixes: 278f945402c5 ("pdump: add new library for packet capture")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 lib/librte_pdump/rte_pdump.c | 44 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index 5c335ba..8240387 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -442,7 +442,7 @@ set_pdump_rxtx_cbs(struct pdump_request *p)
 }
 
 /* get socket path (/var/run if root, $HOME otherwise) */
-static void
+static int
 pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
 {
 	char dir[PATH_MAX] = {0};
@@ -455,9 +455,17 @@ pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
 	else {
 		if (getuid() != 0) {
 			dir_home = getenv(SOCKET_PATH_HOME);
+			if (!dir_home) {
+				RTE_LOG(ERR, PDUMP,
+					"Failed to get environment variable"
+					" value for %s, %s:%d\n",
+					SOCKET_PATH_HOME, __func__, __LINE__);
+				return -1;
+			}
 			strcat(dir, dir_home);
 		} else
 			strcat(dir, SOCKET_PATH_VAR_RUN);
+
 		strcat(dir, SOCKET_DIR);
 	}
 
@@ -467,6 +475,8 @@ pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
 	else
 		snprintf(buffer, bufsz, CLIENT_SOCKET, dir, getpid(),
 				rte_sys_gettid());
+
+	return 0;
 }
 
 static int
@@ -476,8 +486,14 @@ pdump_create_server_socket(void)
 	struct sockaddr_un addr;
 	socklen_t addr_len;
 
-	pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
+	ret = pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
 				RTE_PDUMP_SOCKET_SERVER);
+	if (ret != 0) {
+		RTE_LOG(ERR, PDUMP,
+			"Failed to get server socket path: %s:%d\n",
+			__func__, __LINE__);
+		return -1;
+	}
 	addr.sun_family = AF_UNIX;
 
 	/* remove if file already exists */
@@ -608,8 +624,14 @@ rte_pdump_uninit(void)
 
 	struct sockaddr_un addr;
 
-	pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
+	ret = pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
 				RTE_PDUMP_SOCKET_SERVER);
+	if (ret != 0) {
+		RTE_LOG(ERR, PDUMP,
+			"Failed to get server socket path: %s:%d\n",
+			__func__, __LINE__);
+		return -1;
+	}
 	ret = unlink(addr.sun_path);
 	if (ret != 0) {
 		RTE_LOG(ERR, PDUMP,
@@ -643,8 +665,14 @@ pdump_create_client_socket(struct pdump_request *p)
 		return ret;
 	}
 
-	pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
+	ret = pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
 				RTE_PDUMP_SOCKET_CLIENT);
+	if (ret != 0) {
+		RTE_LOG(ERR, PDUMP,
+			"Failed to get client socket path: %s:%d\n",
+			__func__, __LINE__);
+		return -1;
+	}
 	addr.sun_family = AF_UNIX;
 	addr_len = sizeof(struct sockaddr_un);
 
@@ -660,9 +688,15 @@ pdump_create_client_socket(struct pdump_request *p)
 
 		serv_len = sizeof(struct sockaddr_un);
 		memset(&serv_addr, 0, sizeof(serv_addr));
-		pdump_get_socket_path(serv_addr.sun_path,
+		ret = pdump_get_socket_path(serv_addr.sun_path,
 					sizeof(serv_addr.sun_path),
 					RTE_PDUMP_SOCKET_SERVER);
+		if (ret != 0) {
+			RTE_LOG(ERR, PDUMP,
+				"Failed to get server socket path: %s:%d\n",
+				__func__, __LINE__);
+			break;
+		}
 		serv_addr.sun_family = AF_UNIX;
 
 		n =  sendto(socket_fd, p, sizeof(struct pdump_request), 0,
-- 
2.5.0

[dpdk-dev] [PATCH v4 3/5] pdump: fix string overflow

[Reshma Pattan]

replaced strncpy with snprintf for safely
copying the strings.

Cverity issue 127350: string overflow

Fixes: 278f945402c5 ("pdump: add new library for packet capture")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 lib/librte_pdump/rte_pdump.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index 8240387..53a5bf2 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -804,13 +804,15 @@ pdump_prepare_client_request(char *device, uint16_t queue,
 	req.flags = flags;
 	req.op =  operation;
 	if ((operation & ENABLE) != 0) {
-		strncpy(req.data.en_v1.device, device, strlen(device));
+		snprintf(req.data.en_v1.device, sizeof(req.data.en_v1.device),
+				"%s", device);
 		req.data.en_v1.queue = queue;
 		req.data.en_v1.ring = ring;
 		req.data.en_v1.mp = mp;
 		req.data.en_v1.filter = filter;
 	} else {
-		strncpy(req.data.dis_v1.device, device, strlen(device));
+		snprintf(req.data.dis_v1.device, sizeof(req.data.dis_v1.device),
+				"%s", device);
 		req.data.dis_v1.queue = queue;
 		req.data.dis_v1.ring = NULL;
 		req.data.dis_v1.mp = NULL;
-- 
2.5.0

[dpdk-dev] [PATCH v4 4/5] app/pdump: fix string overflow

[Reshma Pattan]

replaced strncpy with snprintf for safely
copying the strings.

Coverity issue 127351: string overflow

Fixes: caa7028276b8 ("app/pdump: add tool for packet capturing")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 app/pdump/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/pdump/main.c b/app/pdump/main.c
index f8923b9..fe4d38a 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -217,12 +217,12 @@ parse_rxtxdev(const char *key, const char *value, void *extra_args)
 	struct pdump_tuples *pt = extra_args;
 
 	if (!strcmp(key, PDUMP_RX_DEV_ARG)) {
-		strncpy(pt->rx_dev, value, strlen(value));
+		snprintf(pt->rx_dev, sizeof(pt->rx_dev), "%s", value);
 		/* identify the tx stream type for pcap vdev */
 		if (if_nametoindex(pt->rx_dev))
 			pt->rx_vdev_stream_type = IFACE;
 	} else if (!strcmp(key, PDUMP_TX_DEV_ARG)) {
-		strncpy(pt->tx_dev, value, strlen(value));
+		snprintf(pt->tx_dev, sizeof(pt->tx_dev), "%s", value);
 		/* identify the tx stream type for pcap vdev */
 		if (if_nametoindex(pt->tx_dev))
 			pt->tx_vdev_stream_type = IFACE;
-- 
2.5.0

[dpdk-dev] [PATCH v4 5/5] app/pdump: fix type casting of ring size

[Reshma Pattan]

ring_size value is wrongly type casted to uint16_t.
It should be type casted to uint32_t, as maximum
ring size is 28bit long. Wrong type cast
wrapping around the ring size values bigger than 65535.

Fixes: caa7028276b8 ("app/pdump: add tool for packet capturing")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 app/pdump/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/pdump/main.c b/app/pdump/main.c
index fe4d38a..2087c15 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -362,7 +362,7 @@ parse_pdump(const char *optarg)
 						&parse_uint_value, &v);
 		if (ret < 0)
 			goto free_kvlist;
-		pt->ring_size = (uint16_t) v.val;
+		pt->ring_size = (uint32_t) v.val;
 	} else
 		pt->ring_size = RING_SIZE;
 
-- 
2.5.0

[dpdk-dev] [PATCH v5 0/5] fix issues in packet capture framework

[Reshma Pattan]

This patchset includes listed fixes
1)fix default socket path in pdump library.
2)fix coverity issues in pdump library.
3)fix coverity issues in pdump tool.
4)fix wrong typecast of ring size in pdump tool.

v5:
changes are done to default socket paths
now default socket path will be /var/run/.dpdk/pdump_sockets
for root users and HOME/.dpdk/pdump_sockets for nonroot users.

v4:
added new patch for fixing wrong typecast of ring size
in pdump tool.

v3:
added new patch for fixing default socket paths "HOME" and "/var/run".
reworked coverity fixes on top of the above change.

v2:
fixed code review comment to use snprintf instead of strncpy.


Reshma Pattan (5):
  pdump: fix default socket path
  pdump: check getenv return value
  pdump: fix string overflow
  app/pdump: fix string overflow
  app/pdump: fix type casting of ring size

 app/pdump/main.c             |  6 ++--
 lib/librte_pdump/rte_pdump.c | 78 +++++++++++++++++++++++++++++++++++---------
 2 files changed, 65 insertions(+), 19 deletions(-)

-- 
2.5.0

[dpdk-dev] [PATCH v5 1/5] pdump: fix default socket path

[Reshma Pattan]

SOCKET_PATH_HOME is to specify environment variable "HOME",
so it should not contain "/pdump_sockets"  in the macro.
So removed "/pdump_sockets" from SOCKET_PATH_HOME and
SOCKET_PATH_VAR_RUN. New changes will create pdump sockets under
/var/run/.dpdk/pdump_sockets for root users and
under HOME/.dpdk/pdump_sockets for non root users.
Changes are done in pdump_get_socket_path() to accommodate
new socket path changes.

Fixes: 278f945402c5 ("pdump: add new library for packet capture")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 lib/librte_pdump/rte_pdump.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index c921f51..70efd96 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -50,8 +50,10 @@
 
 #include "rte_pdump.h"
 
-#define SOCKET_PATH_VAR_RUN "/var/run/pdump_sockets"
-#define SOCKET_PATH_HOME "HOME/pdump_sockets"
+#define SOCKET_PATH_VAR_RUN "/var/run"
+#define SOCKET_PATH_HOME "HOME"
+#define DPDK_DIR         "/.dpdk"
+#define SOCKET_DIR       "/pdump_sockets"
 #define SERVER_SOCKET "%s/pdump_server_socket"
 #define CLIENT_SOCKET "%s/pdump_client_socket_%d_%u"
 #define DEVICE_ID_SIZE 64
@@ -444,17 +446,26 @@ set_pdump_rxtx_cbs(struct pdump_request *p)
 static void
 pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
 {
-	const char *dir = NULL;
+	char dpdk_dir[PATH_MAX] = {0};
+	char dir[PATH_MAX] = {0};
+	char *dir_home = NULL;
 
 	if (type == RTE_PDUMP_SOCKET_SERVER && server_socket_dir[0] != 0)
-		dir = server_socket_dir;
+		snprintf(dir, sizeof(dir), "%s", server_socket_dir);
 	else if (type == RTE_PDUMP_SOCKET_CLIENT && client_socket_dir[0] != 0)
-		dir = client_socket_dir;
+		snprintf(dir, sizeof(dir), "%s", client_socket_dir);
 	else {
-		if (getuid() != 0)
-			dir = getenv(SOCKET_PATH_HOME);
-		else
-			dir = SOCKET_PATH_VAR_RUN;
+		if (getuid() != 0) {
+			dir_home = getenv(SOCKET_PATH_HOME);
+			snprintf(dpdk_dir, sizeof(dpdk_dir), "%s%s",
+					dir_home, DPDK_DIR);
+		} else
+			snprintf(dpdk_dir, sizeof(dpdk_dir), "%s%s",
+					SOCKET_PATH_VAR_RUN, DPDK_DIR);
+
+		mkdir(dpdk_dir, 700);
+		snprintf(dir, sizeof(dir), "%s%s",
+					dpdk_dir, SOCKET_DIR);
 	}
 
 	mkdir(dir, 700);
-- 
2.5.0

Re: [dpdk-dev] [PATCH v5 1/5] pdump: fix default socket path

[Mcnamara, John]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Reshma Pattan
> Sent: Friday, June 24, 2016 5:36 PM
> To: dev@dpdk.org
> Cc: Pattan, Reshma <reshma.pattan@intel.com>
> Subject: [dpdk-dev] [PATCH v5 1/5] pdump: fix default socket path
> 
> SOCKET_PATH_HOME is to specify environment variable "HOME", so it should
> not contain "/pdump_sockets"  in the macro.
> So removed "/pdump_sockets" from SOCKET_PATH_HOME and SOCKET_PATH_VAR_RUN.
> New changes will create pdump sockets under /var/run/.dpdk/pdump_sockets
> for root users and under HOME/.dpdk/pdump_sockets for non root users.
> Changes are done in pdump_get_socket_path() to accommodate new socket path
> changes.
> 
> Fixes: 278f945402c5 ("pdump: add new library for packet capture")
> 
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>

Acked-by: John McNamara <john.mcnamara@intel.com>

[dpdk-dev] [PATCH v5 2/5] pdump: check getenv return value

[Reshma Pattan]

inside pdump_get_socket_path(), getenv can return
a NULL pointer if the match for SOCKET_PATH_HOME is
not found in the environment. NULL check is added to
return -1 immediately. Since pdump_get_socket_path()
returns -1 now, wherever this function is called
there the return value is checked and error message
is logged.

Coverity issue 127344:  return value check
Coverity issue 127347:  null pointer dereference

Fixes: 278f945402c5 ("pdump: add new library for packet capture")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 lib/librte_pdump/rte_pdump.c | 43 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index 70efd96..e3b03a6 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -443,7 +443,7 @@ set_pdump_rxtx_cbs(struct pdump_request *p)
 }
 
 /* get socket path (/var/run if root, $HOME otherwise) */
-static void
+static int
 pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
 {
 	char dpdk_dir[PATH_MAX] = {0};
@@ -457,6 +457,13 @@ pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
 	else {
 		if (getuid() != 0) {
 			dir_home = getenv(SOCKET_PATH_HOME);
+			if (!dir_home) {
+				RTE_LOG(ERR, PDUMP,
+					"Failed to get environment variable"
+					" value for %s, %s:%d\n",
+					SOCKET_PATH_HOME, __func__, __LINE__);
+				return -1;
+			}
 			snprintf(dpdk_dir, sizeof(dpdk_dir), "%s%s",
 					dir_home, DPDK_DIR);
 		} else
@@ -474,6 +481,8 @@ pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
 	else
 		snprintf(buffer, bufsz, CLIENT_SOCKET, dir, getpid(),
 				rte_sys_gettid());
+
+	return 0;
 }
 
 static int
@@ -483,8 +492,14 @@ pdump_create_server_socket(void)
 	struct sockaddr_un addr;
 	socklen_t addr_len;
 
-	pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
+	ret = pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
 				RTE_PDUMP_SOCKET_SERVER);
+	if (ret != 0) {
+		RTE_LOG(ERR, PDUMP,
+			"Failed to get server socket path: %s:%d\n",
+			__func__, __LINE__);
+		return -1;
+	}
 	addr.sun_family = AF_UNIX;
 
 	/* remove if file already exists */
@@ -615,8 +630,14 @@ rte_pdump_uninit(void)
 
 	struct sockaddr_un addr;
 
-	pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
+	ret = pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
 				RTE_PDUMP_SOCKET_SERVER);
+	if (ret != 0) {
+		RTE_LOG(ERR, PDUMP,
+			"Failed to get server socket path: %s:%d\n",
+			__func__, __LINE__);
+		return -1;
+	}
 	ret = unlink(addr.sun_path);
 	if (ret != 0) {
 		RTE_LOG(ERR, PDUMP,
@@ -650,8 +671,14 @@ pdump_create_client_socket(struct pdump_request *p)
 		return ret;
 	}
 
-	pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
+	ret = pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
 				RTE_PDUMP_SOCKET_CLIENT);
+	if (ret != 0) {
+		RTE_LOG(ERR, PDUMP,
+			"Failed to get client socket path: %s:%d\n",
+			__func__, __LINE__);
+		return -1;
+	}
 	addr.sun_family = AF_UNIX;
 	addr_len = sizeof(struct sockaddr_un);
 
@@ -667,9 +694,15 @@ pdump_create_client_socket(struct pdump_request *p)
 
 		serv_len = sizeof(struct sockaddr_un);
 		memset(&serv_addr, 0, sizeof(serv_addr));
-		pdump_get_socket_path(serv_addr.sun_path,
+		ret = pdump_get_socket_path(serv_addr.sun_path,
 					sizeof(serv_addr.sun_path),
 					RTE_PDUMP_SOCKET_SERVER);
+		if (ret != 0) {
+			RTE_LOG(ERR, PDUMP,
+				"Failed to get server socket path: %s:%d\n",
+				__func__, __LINE__);
+			break;
+		}
 		serv_addr.sun_family = AF_UNIX;
 
 		n =  sendto(socket_fd, p, sizeof(struct pdump_request), 0,
-- 
2.5.0

Re: [dpdk-dev] [PATCH v5 2/5] pdump: check getenv return value

[Mcnamara, John]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Reshma Pattan
> Sent: Friday, June 24, 2016 5:36 PM
> To: dev@dpdk.org
> Cc: Pattan, Reshma <reshma.pattan@intel.com>
> Subject: [dpdk-dev] [PATCH v5 2/5] pdump: check getenv return value
> 
> inside pdump_get_socket_path(), getenv can return a NULL pointer if the
> match for SOCKET_PATH_HOME is not found in the environment. NULL check is
> added to return -1 immediately. Since pdump_get_socket_path() returns -1
> now, wherever this function is called there the return value is checked
> and error message is logged.
> 
> Coverity issue 127344:  return value check Coverity issue 127347:  null
> pointer dereference
> 
> Fixes: 278f945402c5 ("pdump: add new library for packet capture")
> 
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>

Acked-by: John McNamara <john.mcnamara@intel.com>

[dpdk-dev] [PATCH v5 3/5] pdump: fix string overflow

[Reshma Pattan]

replaced strncpy with snprintf for safely
copying the strings.

Cverity issue 127350: string overflow

Fixes: 278f945402c5 ("pdump: add new library for packet capture")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 lib/librte_pdump/rte_pdump.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index e3b03a6..ee566cb 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -810,13 +810,15 @@ pdump_prepare_client_request(char *device, uint16_t queue,
 	req.flags = flags;
 	req.op =  operation;
 	if ((operation & ENABLE) != 0) {
-		strncpy(req.data.en_v1.device, device, strlen(device));
+		snprintf(req.data.en_v1.device, sizeof(req.data.en_v1.device),
+				"%s", device);
 		req.data.en_v1.queue = queue;
 		req.data.en_v1.ring = ring;
 		req.data.en_v1.mp = mp;
 		req.data.en_v1.filter = filter;
 	} else {
-		strncpy(req.data.dis_v1.device, device, strlen(device));
+		snprintf(req.data.dis_v1.device, sizeof(req.data.dis_v1.device),
+				"%s", device);
 		req.data.dis_v1.queue = queue;
 		req.data.dis_v1.ring = NULL;
 		req.data.dis_v1.mp = NULL;
-- 
2.5.0

Re: [dpdk-dev] [PATCH v5 3/5] pdump: fix string overflow

[Mcnamara, John]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Reshma Pattan
> Sent: Friday, June 24, 2016 5:36 PM
> To: dev@dpdk.org
> Cc: Pattan, Reshma <reshma.pattan@intel.com>
> Subject: [dpdk-dev] [PATCH v5 3/5] pdump: fix string overflow
> 
> replaced strncpy with snprintf for safely copying the strings.
> 
> Cverity issue 127350: string overflow
> 
> Fixes: 278f945402c5 ("pdump: add new library for packet capture")
> 
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>

Acked-by: John McNamara <john.mcnamara@intel.com>

[dpdk-dev] [PATCH v5 4/5] app/pdump: fix string overflow

[Reshma Pattan]

replaced strncpy with snprintf for safely
copying the strings.

Coverity issue 127351: string overflow

Fixes: caa7028276b8 ("app/pdump: add tool for packet capturing")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 app/pdump/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/pdump/main.c b/app/pdump/main.c
index f8923b9..fe4d38a 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -217,12 +217,12 @@ parse_rxtxdev(const char *key, const char *value, void *extra_args)
 	struct pdump_tuples *pt = extra_args;
 
 	if (!strcmp(key, PDUMP_RX_DEV_ARG)) {
-		strncpy(pt->rx_dev, value, strlen(value));
+		snprintf(pt->rx_dev, sizeof(pt->rx_dev), "%s", value);
 		/* identify the tx stream type for pcap vdev */
 		if (if_nametoindex(pt->rx_dev))
 			pt->rx_vdev_stream_type = IFACE;
 	} else if (!strcmp(key, PDUMP_TX_DEV_ARG)) {
-		strncpy(pt->tx_dev, value, strlen(value));
+		snprintf(pt->tx_dev, sizeof(pt->tx_dev), "%s", value);
 		/* identify the tx stream type for pcap vdev */
 		if (if_nametoindex(pt->tx_dev))
 			pt->tx_vdev_stream_type = IFACE;
-- 
2.5.0

Re: [dpdk-dev] [PATCH v5 4/5] app/pdump: fix string overflow

[Mcnamara, John]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Reshma Pattan
> Sent: Friday, June 24, 2016 5:36 PM
> To: dev@dpdk.org
> Cc: Pattan, Reshma <reshma.pattan@intel.com>
> Subject: [dpdk-dev] [PATCH v5 4/5] app/pdump: fix string overflow
> 
> replaced strncpy with snprintf for safely copying the strings.
> 
> Coverity issue 127351: string overflow
> 
> Fixes: caa7028276b8 ("app/pdump: add tool for packet capturing")
> 
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>

Acked-by: John McNamara <john.mcnamara@intel.com>

[dpdk-dev] [PATCH v5 5/5] app/pdump: fix type casting of ring size

[Reshma Pattan]

ring_size value is wrongly type casted to uint16_t.
It should be type casted to uint32_t, as maximum
ring size is 28bit long. Wrong type cast
wrapping around the ring size values bigger than 65535.

Fixes: caa7028276b8 ("app/pdump: add tool for packet capturing")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 app/pdump/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/pdump/main.c b/app/pdump/main.c
index fe4d38a..2087c15 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -362,7 +362,7 @@ parse_pdump(const char *optarg)
 						&parse_uint_value, &v);
 		if (ret < 0)
 			goto free_kvlist;
-		pt->ring_size = (uint16_t) v.val;
+		pt->ring_size = (uint32_t) v.val;
 	} else
 		pt->ring_size = RING_SIZE;
 
-- 
2.5.0

Re: [dpdk-dev] [PATCH v5 5/5] app/pdump: fix type casting of ring size

[Mcnamara, John]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Reshma Pattan
> Sent: Friday, June 24, 2016 5:36 PM
> To: dev@dpdk.org
> Cc: Pattan, Reshma <reshma.pattan@intel.com>
> Subject: [dpdk-dev] [PATCH v5 5/5] app/pdump: fix type casting of ring
> size
> 
> ring_size value is wrongly type casted to uint16_t.
> It should be type casted to uint32_t, as maximum ring size is 28bit long.
> Wrong type cast wrapping around the ring size values bigger than 65535.
> 
> Fixes: caa7028276b8 ("app/pdump: add tool for packet capturing")
> 
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>

Acked-by: John McNamara <john.mcnamara@intel.com>

Re: [dpdk-dev] [PATCH v5 0/5] fix issues in packet capture framework

[Thomas Monjalon]

> Reshma Pattan (5):
>   pdump: fix default socket path
>   pdump: check getenv return value
>   pdump: fix string overflow
>   app/pdump: fix string overflow
>   app/pdump: fix type casting of ring size

Applied, thanks