diff mbox

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

Message ID 1466522285-15023-2-git-send-email-reshma.pattan@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Pattan, Reshma June 21, 2016, 3:18 p.m. UTC 
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(-)

Comments

Ferruh Yigit June 21, 2016, 4:55 p.m. UTC | #1 
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?
Pattan, Reshma June 22, 2016, 8:01 a.m. UTC | #2 
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
diff mbox

Patch

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,