[dpdk-dev] [PATCH v2] net/memif: fix invalid unix domain address length

[Jakub Grajciar <jgrajcia@cisco.com>]

Define MEMIF_SOCKET_UN_SIZE to size of unix domain socket address.
Report error in case of longer path.

Fixes: b923866c6974 ("net/memif: allow for full key size in socket name")
Cc: stephen@networkplumber.org

Signed-off-by: Jakub Grajciar <jgrajcia@cisco.com>
---
 doc/guides/nics/memif.rst         |  2 +-
 drivers/net/memif/memif_socket.c  | 27 +++++++++++----------------
 drivers/net/memif/memif_socket.h  |  6 ++++--
 drivers/net/memif/rte_eth_memif.c |  5 +++++
 4 files changed, 21 insertions(+), 19 deletions(-)

v2:
- fix coding style
- fix socket path length check

diff --git a/doc/guides/nics/memif.rst b/doc/guides/nics/memif.rst
index de2d481eb..9a568455e 100644
--- a/doc/guides/nics/memif.rst
+++ b/doc/guides/nics/memif.rst
@@ -42,7 +42,7 @@ client.
    "role=master", "Set memif role", "slave", "master|slave"
    "bsize=1024", "Size of single packet buffer", "2048", "uint16_t"
    "rsize=11", "Log2 of ring size. If rsize is 10, actual ring size is 1024", "10", "1-14"
-   "socket=/tmp/memif.sock", "Socket filename", "/tmp/memif.sock", "string len 256"
+   "socket=/tmp/memif.sock", "Socket filename", "/tmp/memif.sock", "string len 108"
    "mac=01:23:45:ab:cd:ef", "Mac address", "01:ab:23:cd:45:ef", ""
    "secret=abc123", "Secret is an optional security option, which if specified, must be matched by peer", "", "string len 24"
    "zero-copy=yes", "Enable/disable zero-copy slave mode", "no", "yes|no"
diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index 0c71f6c45..4efa68e1a 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -7,7 +7,6 @@
 #include <unistd.h>
 #include <sys/types.h>
 #include <sys/socket.h>
-#include <sys/un.h>
 #include <sys/ioctl.h>
 #include <errno.h>

@@ -860,16 +859,12 @@ memif_listener_handler(void *arg)
 		rte_free(cc);
 }

-#define MEMIF_SOCKET_UN_SIZE	\
-	(offsetof(struct sockaddr_un, sun_path) + MEMIF_SOCKET_KEY_LEN)
-
 static struct memif_socket *
 memif_socket_create(struct pmd_internals *pmd,
 		    const char *key, uint8_t listener)
 {
 	struct memif_socket *sock;
-	struct sockaddr_un *un;
-	char un_buf[MEMIF_SOCKET_UN_SIZE];
+	struct sockaddr_un un;
 	int sockfd;
 	int ret;
 	int on = 1;
@@ -881,7 +876,7 @@ memif_socket_create(struct pmd_internals *pmd,
 	}

 	sock->listener = listener;
-	strlcpy(sock->filename, key, MEMIF_SOCKET_KEY_LEN);
+	strlcpy(sock->filename, key, MEMIF_SOCKET_UN_SIZE);
 	TAILQ_INIT(&sock->dev_queue);

 	if (listener != 0) {
@@ -889,18 +884,18 @@ memif_socket_create(struct pmd_internals *pmd,
 		if (sockfd < 0)
 			goto error;

-		memset(un_buf, 0, sizeof(un_buf));
-		un = (struct sockaddr_un *)un_buf;
-		un->sun_family = AF_UNIX;
-		strlcpy(un->sun_path, sock->filename, MEMIF_SOCKET_KEY_LEN);
+		un.sun_family = AF_UNIX;
+		strlcpy(un.sun_path, sock->filename, MEMIF_SOCKET_UN_SIZE);

 		ret = setsockopt(sockfd, SOL_SOCKET, SO_PASSCRED, &on,
 				 sizeof(on));
 		if (ret < 0)
 			goto error;
-		ret = bind(sockfd, (struct sockaddr *)un, MEMIF_SOCKET_UN_SIZE);
+
+		ret = bind(sockfd, (struct sockaddr *)&un, sizeof(un));
 		if (ret < 0)
 			goto error;
+
 		ret = listen(sockfd, 1);
 		if (ret < 0)
 			goto error;
@@ -940,7 +935,7 @@ memif_create_socket_hash(void)

 	params.name = MEMIF_SOCKET_HASH_NAME;
 	params.entries = 256;
-	params.key_len = MEMIF_SOCKET_KEY_LEN;
+	params.key_len = MEMIF_SOCKET_UN_SIZE;
 	params.hash_func = rte_jhash;
 	params.hash_func_init_val = 0;
 	return rte_hash_create(&params);
@@ -955,7 +950,7 @@ memif_socket_init(struct rte_eth_dev *dev, const char *socket_filename)
 	struct pmd_internals *tmp_pmd;
 	struct rte_hash *hash;
 	int ret;
-	char key[MEMIF_SOCKET_KEY_LEN];
+	char key[MEMIF_SOCKET_UN_SIZE];

 	hash = rte_hash_find_existing(MEMIF_SOCKET_HASH_NAME);
 	if (hash == NULL) {
@@ -966,8 +961,8 @@ memif_socket_init(struct rte_eth_dev *dev, const char *socket_filename)
 		}
 	}

-	memset(key, 0, MEMIF_SOCKET_KEY_LEN);
-	strlcpy(key, socket_filename, MEMIF_SOCKET_KEY_LEN);
+	memset(key, 0, MEMIF_SOCKET_UN_SIZE);
+	strlcpy(key, socket_filename, MEMIF_SOCKET_UN_SIZE);
 	ret = rte_hash_lookup_data(hash, key, (void **)&socket);
 	if (ret < 0) {
 		socket = memif_socket_create(pmd, key,
diff --git a/drivers/net/memif/memif_socket.h b/drivers/net/memif/memif_socket.h
index 9f40f8d13..5c49ec24e 100644
--- a/drivers/net/memif/memif_socket.h
+++ b/drivers/net/memif/memif_socket.h
@@ -6,6 +6,7 @@
 #define _MEMIF_SOCKET_H_

 #include <sys/queue.h>
+#include <sys/un.h>

 /**
  * Remove device from socket device list. If no device is left on the socket,
@@ -79,11 +80,12 @@ struct memif_socket_dev_list_elt {
 };

 #define MEMIF_SOCKET_HASH_NAME			"memif-sh"
-#define MEMIF_SOCKET_KEY_LEN		256
+#define MEMIF_SOCKET_UN_SIZE	\
+	(sizeof(struct sockaddr_un) - offsetof(struct sockaddr_un, sun_path))

 struct memif_socket {
 	struct rte_intr_handle intr_handle;	/**< interrupt handle */
-	char filename[MEMIF_SOCKET_KEY_LEN];	/**< socket filename */
+	char filename[MEMIF_SOCKET_UN_SIZE];	/**< socket filename */

 	TAILQ_HEAD(, memif_socket_dev_list_elt) dev_queue;
 	/**< Queue of devices using this socket */
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index a347e27bd..d52db91ed 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -1192,6 +1192,11 @@ memif_check_socket_filename(const char *filename)
 	uint32_t idx;
 	int ret = 0;

+	if (strlen(filename) >= MEMIF_SOCKET_UN_SIZE) {
+		MIF_LOG(ERR, "Unix socket address too long (max 108).");
+		return -1;
+	}
+
 	tmp = strrchr(filename, '/');
 	if (tmp != NULL) {
 		idx = tmp - filename;
--
2.17.1