[RFC PATCH] net/memif: change socket listener owner uid/gid

[RFC PATCH] net/memif: change socket listener owner uid/gid

[Junxiao Shi]

This allows a DPDK application running with root privilege to create a
memif socket listener with non-root owner uid and gid, which can be
connected from client applications running without root privilege.

Signed-off-by: Junxiao Shi <git@mail1.yoursunny.com>
---
 doc/guides/nics/memif.rst         |  2 ++
 drivers/net/memif/memif_socket.c  | 13 +++++--
 drivers/net/memif/rte_eth_memif.c | 56 +++++++++++++++++++++++++------
 drivers/net/memif/rte_eth_memif.h |  2 ++
 4 files changed, 60 insertions(+), 13 deletions(-)

diff --git a/doc/guides/nics/memif.rst b/doc/guides/nics/memif.rst
index aca843640b..8a8141aa72 100644
--- a/doc/guides/nics/memif.rst
+++ b/doc/guides/nics/memif.rst
@@ -44,6 +44,8 @@ client.
    "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 108"
    "socket-abstract=no", "Set usage of abstract socket address", "yes", "yes|no"
+   "uid=1000", "Set socket listener owner uid. Only relevant to server with socket-abstract=no", "unchanged", "uid_t"
+   "gid=1000", "Set socket listener owner gid. Only relevant to server with socket-abstract=no", "unchanged", "gid_t"
    "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 client mode. Only relevant to client, requires '--single-file-segments' eal argument", "no", "yes|no"
diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index 7886644412..bedb0637a9 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -889,7 +889,7 @@ memif_listener_handler(void *arg)
 }
 
 static struct memif_socket *
-memif_socket_create(char *key, uint8_t listener, bool is_abstract)
+memif_socket_create(char *key, uint8_t listener, bool is_abstract, uid_t owner_uid, gid_t owner_gid)
 {
 	struct memif_socket *sock;
 	struct sockaddr_un un = { 0 };
@@ -941,6 +941,14 @@ memif_socket_create(char *key, uint8_t listener, bool is_abstract)
 
 		MIF_LOG(DEBUG, "Memif listener socket %s created.", sock->filename);
 
+		if (!is_abstract && (owner_uid != (uid_t)-1 || owner_gid != (gid_t)-1)) {
+			ret = chown(sock->filename, owner_uid, owner_gid);
+			if (ret < 0) {
+				MIF_LOG(ERR, "Failed to change listener socket owner %d", errno);
+				goto error;
+			}
+		}
+
 		/* Allocate interrupt instance */
 		sock->intr_handle =
 			rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_SHARED);
@@ -1017,7 +1025,8 @@ memif_socket_init(struct rte_eth_dev *dev, const char *socket_filename)
 	if (ret < 0) {
 		socket = memif_socket_create(key,
 			(pmd->role == MEMIF_ROLE_CLIENT) ? 0 : 1,
-			pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT);
+			pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT,
+			pmd->owner_uid, pmd->owner_gid);
 		if (socket == NULL)
 			return -1;
 		ret = rte_hash_add_key_data(hash, key, socket);
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index dd951b8296..f72a53bc03 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -37,6 +37,8 @@
 #define ETH_MEMIF_RING_SIZE_ARG		"rsize"
 #define ETH_MEMIF_SOCKET_ARG		"socket"
 #define ETH_MEMIF_SOCKET_ABSTRACT_ARG	"socket-abstract"
+#define ETH_MEMIF_OWNER_UID_ARG		"owner-uid"
+#define ETH_MEMIF_OWNER_GID_ARG		"owner-gid"
 #define ETH_MEMIF_MAC_ARG		"mac"
 #define ETH_MEMIF_ZC_ARG		"zero-copy"
 #define ETH_MEMIF_SECRET_ARG		"secret"
@@ -48,6 +50,8 @@ static const char * const valid_arguments[] = {
 	ETH_MEMIF_RING_SIZE_ARG,
 	ETH_MEMIF_SOCKET_ARG,
 	ETH_MEMIF_SOCKET_ABSTRACT_ARG,
+	ETH_MEMIF_OWNER_UID_ARG,
+	ETH_MEMIF_OWNER_GID_ARG,
 	ETH_MEMIF_MAC_ARG,
 	ETH_MEMIF_ZC_ARG,
 	ETH_MEMIF_SECRET_ARG,
@@ -1515,7 +1519,7 @@ static const struct eth_dev_ops ops = {
 static int
 memif_create(struct rte_vdev_device *vdev, enum memif_role_t role,
 	     memif_interface_id_t id, uint32_t flags,
-	     const char *socket_filename,
+	     const char *socket_filename, uid_t owner_uid, gid_t owner_gid,
 	     memif_log2_ring_size_t log2_ring_size,
 	     uint16_t pkt_buffer_size, const char *secret,
 	     struct rte_ether_addr *ether_addr)
@@ -1554,6 +1558,8 @@ memif_create(struct rte_vdev_device *vdev, enum memif_role_t role,
 	/* Zero-copy flag irelevant to server. */
 	if (pmd->role == MEMIF_ROLE_SERVER)
 		pmd->flags &= ~ETH_MEMIF_FLAG_ZERO_COPY;
+	pmd->owner_uid = owner_uid;
+	pmd->owner_gid = owner_gid;
 
 	ret = memif_socket_init(eth_dev, socket_filename);
 	if (ret < 0)
@@ -1740,6 +1746,22 @@ memif_set_is_socket_abstract(const char *key __rte_unused, const char *value, vo
 	return 0;
 }
 
+static int
+memif_set_owner_uid(const char *key __rte_unused, const char *value, void *extra_args)
+{
+	uid_t *uid = (uid_t *)extra_args;
+	*uid = strtoul(value, NULL, 10);
+	return 0;
+}
+
+static int
+memif_set_owner_gid(const char *key __rte_unused, const char *value, void *extra_args)
+{
+	gid_t *gid = (gid_t *)extra_args;
+	*gid = strtoul(value, NULL, 10);
+	return 0;
+}
+
 static int
 memif_set_mac(const char *key __rte_unused, const char *value, void *extra_args)
 {
@@ -1772,6 +1794,8 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 	uint16_t pkt_buffer_size = ETH_MEMIF_DEFAULT_PKT_BUFFER_SIZE;
 	memif_log2_ring_size_t log2_ring_size = ETH_MEMIF_DEFAULT_RING_SIZE;
 	const char *socket_filename = ETH_MEMIF_DEFAULT_SOCKET_FILENAME;
+	uid_t owner_uid = -1;
+	gid_t owner_gid = -1;
 	uint32_t flags = 0;
 	const char *secret = NULL;
 	struct rte_ether_addr *ether_addr = rte_zmalloc("",
@@ -1859,6 +1883,14 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 					 &memif_set_mac, ether_addr);
 		if (ret < 0)
 			goto exit;
+		ret = rte_kvargs_process(kvlist, ETH_MEMIF_OWNER_UID_ARG,
+					 &memif_set_owner_uid, &owner_uid);
+		if (ret < 0)
+			goto exit;
+		ret = rte_kvargs_process(kvlist, ETH_MEMIF_OWNER_GID_ARG,
+					 &memif_set_owner_gid, &owner_gid);
+		if (ret < 0)
+			goto exit;
 		ret = rte_kvargs_process(kvlist, ETH_MEMIF_ZC_ARG,
 					 &memif_set_zc, &flags);
 		if (ret < 0)
@@ -1876,7 +1908,7 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 	}
 
 	/* create interface */
-	ret = memif_create(vdev, role, id, flags, socket_filename,
+	ret = memif_create(vdev, role, id, flags, socket_filename, owner_uid, owner_gid,
 			   log2_ring_size, pkt_buffer_size, secret, ether_addr);
 
 exit:
@@ -1904,14 +1936,16 @@ static struct rte_vdev_driver pmd_memif_drv = {
 RTE_PMD_REGISTER_VDEV(net_memif, pmd_memif_drv);
 
 RTE_PMD_REGISTER_PARAM_STRING(net_memif,
-			      ETH_MEMIF_ID_ARG "=<int>"
-			      ETH_MEMIF_ROLE_ARG "=server|client"
-			      ETH_MEMIF_PKT_BUFFER_SIZE_ARG "=<int>"
-			      ETH_MEMIF_RING_SIZE_ARG "=<int>"
-			      ETH_MEMIF_SOCKET_ARG "=<string>"
-				  ETH_MEMIF_SOCKET_ABSTRACT_ARG "=yes|no"
-			      ETH_MEMIF_MAC_ARG "=xx:xx:xx:xx:xx:xx"
-			      ETH_MEMIF_ZC_ARG "=yes|no"
-			      ETH_MEMIF_SECRET_ARG "=<string>");
+						ETH_MEMIF_ID_ARG "=<int>"
+						ETH_MEMIF_ROLE_ARG "=server|client"
+						ETH_MEMIF_PKT_BUFFER_SIZE_ARG "=<int>"
+						ETH_MEMIF_RING_SIZE_ARG "=<int>"
+						ETH_MEMIF_SOCKET_ARG "=<string>"
+						ETH_MEMIF_SOCKET_ABSTRACT_ARG "=yes|no"
+						ETH_MEMIF_OWNER_UID_ARG "=<int>"
+						ETH_MEMIF_OWNER_GID_ARG "=<int>"
+						ETH_MEMIF_MAC_ARG "=xx:xx:xx:xx:xx:xx"
+						ETH_MEMIF_ZC_ARG "=yes|no"
+						ETH_MEMIF_SECRET_ARG "=<string>");
 
 RTE_LOG_REGISTER_DEFAULT(memif_logtype, NOTICE);
diff --git a/drivers/net/memif/rte_eth_memif.h b/drivers/net/memif/rte_eth_memif.h
index 81e7dceae0..b43895a55f 100644
--- a/drivers/net/memif/rte_eth_memif.h
+++ b/drivers/net/memif/rte_eth_memif.h
@@ -89,6 +89,8 @@ struct pmd_internals {
 /**< use abstract socket address */
 
 	char *socket_filename;			/**< pointer to socket filename */
+	uid_t owner_uid;			/**< socket owner uid */
+	gid_t owner_gid;			/**< socket owner gid */
 	char secret[ETH_MEMIF_SECRET_SIZE]; /**< secret (optional security parameter) */
 
 	struct memif_control_channel *cc;	/**< control channel */
-- 
2.17.1

Re: [RFC PATCH] net/memif: change socket listener owner uid/gid

[Stephen Hemminger]

On Tue, 15 Nov 2022 20:44:41 +0000
Junxiao Shi <git@mail1.yoursunny.com> wrote:

> +static int
> +memif_set_owner_uid(const char *key __rte_unused, const char *value, void *extra_args)
> +{
> +	uid_t *uid = (uid_t *)extra_args;
> +	*uid = strtoul(value, NULL, 10);
> +	return 0;
> +}
> +
> +static int
> +memif_set_owner_gid(const char *key __rte_unused, const char *value, void *extra_args)
> +{
> +	gid_t *gid = (gid_t *)extra_args;
> +	*gid = strtoul(value, NULL, 10);
> +	return 0;
> +}
> +

This should be one function and it should check for valid arguments.
Things like not a number, extra characters, and out of range.

Modern Linux allows 32 bits of uid. with some values like -1 reserved.
On a 64 bit system like most DPDK usage, sizeof(unsigned long) is 64 bits.

Since uid and gid have same restrictions, then something like this (untested):

static int strtouid(const char *value, uint32_t *id)
{
	unsigned long val;
	char *endp;

	val = strtoul(value, &endp, 10);
	if (*value == '\0' || *endp != '\0')
		return -EINVAL;
	if (val >= UINT32_MAX)
		return -ERANGE;
	*id = val;
	return 0;
}

[RFC PATCH v2] net/memif: change socket listener owner uid/gid

[Junxiao Shi]

This allows a DPDK application running with root privilege to create a
memif socket listener with non-root owner uid and gid, which can be
connected from client applications running without root privilege.

Signed-off-by: Junxiao Shi <git@mail1.yoursunny.com>
---
 doc/guides/nics/memif.rst         |  2 ++
 drivers/net/memif/memif_socket.c  | 13 +++++++--
 drivers/net/memif/rte_eth_memif.c | 46 +++++++++++++++++++++++++++++--
 drivers/net/memif/rte_eth_memif.h |  2 ++
 4 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/doc/guides/nics/memif.rst b/doc/guides/nics/memif.rst
index aca843640b..8a8141aa72 100644
--- a/doc/guides/nics/memif.rst
+++ b/doc/guides/nics/memif.rst
@@ -44,6 +44,8 @@ client.
    "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 108"
    "socket-abstract=no", "Set usage of abstract socket address", "yes", "yes|no"
+   "uid=1000", "Set socket listener owner uid. Only relevant to server with socket-abstract=no", "unchanged", "uid_t"
+   "gid=1000", "Set socket listener owner gid. Only relevant to server with socket-abstract=no", "unchanged", "gid_t"
    "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 client mode. Only relevant to client, requires '--single-file-segments' eal argument", "no", "yes|no"
diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index 7886644412..c2b038d01a 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -889,7 +889,7 @@ memif_listener_handler(void *arg)
 }
 
 static struct memif_socket *
-memif_socket_create(char *key, uint8_t listener, bool is_abstract)
+memif_socket_create(char *key, uint8_t listener, bool is_abstract, uid_t owner_uid, gid_t owner_gid)
 {
 	struct memif_socket *sock;
 	struct sockaddr_un un = { 0 };
@@ -941,6 +941,14 @@ memif_socket_create(char *key, uint8_t listener, bool is_abstract)
 
 		MIF_LOG(DEBUG, "Memif listener socket %s created.", sock->filename);
 
+		if (!is_abstract && (owner_uid != (uid_t)-1 || owner_gid != (gid_t)-1)) {
+			ret = chown(sock->filename, owner_uid, owner_gid);
+			if (ret < 0) {
+				MIF_LOG(ERR, "Failed to change listener socket owner %d", errno);
+				goto error;
+			}
+		}
+
 		/* Allocate interrupt instance */
 		sock->intr_handle =
 			rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_SHARED);
@@ -1017,7 +1025,8 @@ memif_socket_init(struct rte_eth_dev *dev, const char *socket_filename)
 	if (ret < 0) {
 		socket = memif_socket_create(key,
 			(pmd->role == MEMIF_ROLE_CLIENT) ? 0 : 1,
-			pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT);
+			pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT,
+			pmd->owner_uid, pmd->owner_gid);
 		if (socket == NULL)
 			return -1;
 		ret = rte_hash_add_key_data(hash, key, socket);
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index dd951b8296..d69f0e823f 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -37,6 +37,8 @@
 #define ETH_MEMIF_RING_SIZE_ARG		"rsize"
 #define ETH_MEMIF_SOCKET_ARG		"socket"
 #define ETH_MEMIF_SOCKET_ABSTRACT_ARG	"socket-abstract"
+#define ETH_MEMIF_OWNER_UID_ARG		"owner-uid"
+#define ETH_MEMIF_OWNER_GID_ARG		"owner-gid"
 #define ETH_MEMIF_MAC_ARG		"mac"
 #define ETH_MEMIF_ZC_ARG		"zero-copy"
 #define ETH_MEMIF_SECRET_ARG		"secret"
@@ -48,6 +50,8 @@ static const char * const valid_arguments[] = {
 	ETH_MEMIF_RING_SIZE_ARG,
 	ETH_MEMIF_SOCKET_ARG,
 	ETH_MEMIF_SOCKET_ABSTRACT_ARG,
+	ETH_MEMIF_OWNER_UID_ARG,
+	ETH_MEMIF_OWNER_GID_ARG,
 	ETH_MEMIF_MAC_ARG,
 	ETH_MEMIF_ZC_ARG,
 	ETH_MEMIF_SECRET_ARG,
@@ -1515,7 +1519,7 @@ static const struct eth_dev_ops ops = {
 static int
 memif_create(struct rte_vdev_device *vdev, enum memif_role_t role,
 	     memif_interface_id_t id, uint32_t flags,
-	     const char *socket_filename,
+	     const char *socket_filename, uid_t owner_uid, gid_t owner_gid,
 	     memif_log2_ring_size_t log2_ring_size,
 	     uint16_t pkt_buffer_size, const char *secret,
 	     struct rte_ether_addr *ether_addr)
@@ -1554,6 +1558,8 @@ memif_create(struct rte_vdev_device *vdev, enum memif_role_t role,
 	/* Zero-copy flag irelevant to server. */
 	if (pmd->role == MEMIF_ROLE_SERVER)
 		pmd->flags &= ~ETH_MEMIF_FLAG_ZERO_COPY;
+	pmd->owner_uid = owner_uid;
+	pmd->owner_gid = owner_gid;
 
 	ret = memif_socket_init(eth_dev, socket_filename);
 	if (ret < 0)
@@ -1740,6 +1746,28 @@ memif_set_is_socket_abstract(const char *key __rte_unused, const char *value, vo
 	return 0;
 }
 
+static int
+memif_set_owner(const char *key, const char *value, void *extra_args)
+{
+	static_assert(sizeof(uid_t) == sizeof(uint32_t), "");
+	static_assert(sizeof(gid_t) == sizeof(uint32_t), "");
+	uint32_t *id = (uint32_t *)extra_args;
+
+	char *end = NULL;
+	unsigned long val = strtoul(value, &end, 10);
+	if (*value == '\0' || *end != '\0') {
+		MIF_LOG(ERR, "Failed to parse %s: %s.", key, value);
+		return -EINVAL;
+	}
+	if (val >= UINT32_MAX) {
+		MIF_LOG(ERR, "Invalid %s: %s.", key, value);
+		return -ERANGE;
+	}
+
+	*id = val;
+	return 0;
+}
+
 static int
 memif_set_mac(const char *key __rte_unused, const char *value, void *extra_args)
 {
@@ -1772,6 +1800,8 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 	uint16_t pkt_buffer_size = ETH_MEMIF_DEFAULT_PKT_BUFFER_SIZE;
 	memif_log2_ring_size_t log2_ring_size = ETH_MEMIF_DEFAULT_RING_SIZE;
 	const char *socket_filename = ETH_MEMIF_DEFAULT_SOCKET_FILENAME;
+	uid_t owner_uid = -1;
+	gid_t owner_gid = -1;
 	uint32_t flags = 0;
 	const char *secret = NULL;
 	struct rte_ether_addr *ether_addr = rte_zmalloc("",
@@ -1859,6 +1889,14 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 					 &memif_set_mac, ether_addr);
 		if (ret < 0)
 			goto exit;
+		ret = rte_kvargs_process(kvlist, ETH_MEMIF_OWNER_UID_ARG,
+					 &memif_set_owner, &owner_uid);
+		if (ret < 0)
+			goto exit;
+		ret = rte_kvargs_process(kvlist, ETH_MEMIF_OWNER_GID_ARG,
+					 &memif_set_owner, &owner_gid);
+		if (ret < 0)
+			goto exit;
 		ret = rte_kvargs_process(kvlist, ETH_MEMIF_ZC_ARG,
 					 &memif_set_zc, &flags);
 		if (ret < 0)
@@ -1876,7 +1914,7 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 	}
 
 	/* create interface */
-	ret = memif_create(vdev, role, id, flags, socket_filename,
+	ret = memif_create(vdev, role, id, flags, socket_filename, owner_uid, owner_gid,
 			   log2_ring_size, pkt_buffer_size, secret, ether_addr);
 
 exit:
@@ -1909,7 +1947,9 @@ RTE_PMD_REGISTER_PARAM_STRING(net_memif,
 			      ETH_MEMIF_PKT_BUFFER_SIZE_ARG "=<int>"
 			      ETH_MEMIF_RING_SIZE_ARG "=<int>"
 			      ETH_MEMIF_SOCKET_ARG "=<string>"
-				  ETH_MEMIF_SOCKET_ABSTRACT_ARG "=yes|no"
+			      ETH_MEMIF_SOCKET_ABSTRACT_ARG "=yes|no"
+			      ETH_MEMIF_OWNER_UID_ARG "=<int>"
+			      ETH_MEMIF_OWNER_GID_ARG "=<int>"
 			      ETH_MEMIF_MAC_ARG "=xx:xx:xx:xx:xx:xx"
 			      ETH_MEMIF_ZC_ARG "=yes|no"
 			      ETH_MEMIF_SECRET_ARG "=<string>");
diff --git a/drivers/net/memif/rte_eth_memif.h b/drivers/net/memif/rte_eth_memif.h
index 81e7dceae0..b43895a55f 100644
--- a/drivers/net/memif/rte_eth_memif.h
+++ b/drivers/net/memif/rte_eth_memif.h
@@ -89,6 +89,8 @@ struct pmd_internals {
 /**< use abstract socket address */
 
 	char *socket_filename;			/**< pointer to socket filename */
+	uid_t owner_uid;			/**< socket owner uid */
+	gid_t owner_gid;			/**< socket owner gid */
 	char secret[ETH_MEMIF_SECRET_SIZE]; /**< secret (optional security parameter) */
 
 	struct memif_control_channel *cc;	/**< control channel */
-- 
2.17.1

Re: [RFC PATCH v2] net/memif: change socket listener owner uid/gid

[Stephen Hemminger]

On Wed, 16 Nov 2022 13:09:20 +0000
Junxiao Shi <git@mail1.yoursunny.com> wrote:

> +static int
> +memif_set_owner(const char *key, const char *value, void *extra_args)
> +{
> +	static_assert(sizeof(uid_t) == sizeof(uint32_t), "");
> +	static_assert(sizeof(gid_t) == sizeof(uint32_t), "");
Use RTE_ASSERT() for these
> +	uint32_t *id = (uint32_t *)extra_args;
> +
> +	char *end = NULL;
> +	unsigned long val = strtoul(value, &end, 10);

The DPDK style inherited from Linux kernel is to have blank line
after declarations.

[RFC PATCH v3] net/memif: change socket listener owner uid/gid

[Junxiao Shi]

This allows a DPDK application running with root privilege to create a
memif socket listener with non-root owner uid and gid, which can be
connected from client applications running without root privilege.

Signed-off-by: Junxiao Shi <git@mail1.yoursunny.com>
---
 doc/guides/nics/memif.rst         |  2 ++
 drivers/net/memif/memif_socket.c  | 13 +++++++--
 drivers/net/memif/rte_eth_memif.c | 48 +++++++++++++++++++++++++++++--
 drivers/net/memif/rte_eth_memif.h |  2 ++
 4 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/doc/guides/nics/memif.rst b/doc/guides/nics/memif.rst
index aca843640b..8a8141aa72 100644
--- a/doc/guides/nics/memif.rst
+++ b/doc/guides/nics/memif.rst
@@ -44,6 +44,8 @@ client.
    "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 108"
    "socket-abstract=no", "Set usage of abstract socket address", "yes", "yes|no"
+   "uid=1000", "Set socket listener owner uid. Only relevant to server with socket-abstract=no", "unchanged", "uid_t"
+   "gid=1000", "Set socket listener owner gid. Only relevant to server with socket-abstract=no", "unchanged", "gid_t"
    "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 client mode. Only relevant to client, requires '--single-file-segments' eal argument", "no", "yes|no"
diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index 7886644412..c2b038d01a 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -889,7 +889,7 @@ memif_listener_handler(void *arg)
 }
 
 static struct memif_socket *
-memif_socket_create(char *key, uint8_t listener, bool is_abstract)
+memif_socket_create(char *key, uint8_t listener, bool is_abstract, uid_t owner_uid, gid_t owner_gid)
 {
 	struct memif_socket *sock;
 	struct sockaddr_un un = { 0 };
@@ -941,6 +941,14 @@ memif_socket_create(char *key, uint8_t listener, bool is_abstract)
 
 		MIF_LOG(DEBUG, "Memif listener socket %s created.", sock->filename);
 
+		if (!is_abstract && (owner_uid != (uid_t)-1 || owner_gid != (gid_t)-1)) {
+			ret = chown(sock->filename, owner_uid, owner_gid);
+			if (ret < 0) {
+				MIF_LOG(ERR, "Failed to change listener socket owner %d", errno);
+				goto error;
+			}
+		}
+
 		/* Allocate interrupt instance */
 		sock->intr_handle =
 			rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_SHARED);
@@ -1017,7 +1025,8 @@ memif_socket_init(struct rte_eth_dev *dev, const char *socket_filename)
 	if (ret < 0) {
 		socket = memif_socket_create(key,
 			(pmd->role == MEMIF_ROLE_CLIENT) ? 0 : 1,
-			pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT);
+			pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT,
+			pmd->owner_uid, pmd->owner_gid);
 		if (socket == NULL)
 			return -1;
 		ret = rte_hash_add_key_data(hash, key, socket);
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index dd951b8296..092f1cbc92 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -37,6 +37,8 @@
 #define ETH_MEMIF_RING_SIZE_ARG		"rsize"
 #define ETH_MEMIF_SOCKET_ARG		"socket"
 #define ETH_MEMIF_SOCKET_ABSTRACT_ARG	"socket-abstract"
+#define ETH_MEMIF_OWNER_UID_ARG		"owner-uid"
+#define ETH_MEMIF_OWNER_GID_ARG		"owner-gid"
 #define ETH_MEMIF_MAC_ARG		"mac"
 #define ETH_MEMIF_ZC_ARG		"zero-copy"
 #define ETH_MEMIF_SECRET_ARG		"secret"
@@ -48,6 +50,8 @@ static const char * const valid_arguments[] = {
 	ETH_MEMIF_RING_SIZE_ARG,
 	ETH_MEMIF_SOCKET_ARG,
 	ETH_MEMIF_SOCKET_ABSTRACT_ARG,
+	ETH_MEMIF_OWNER_UID_ARG,
+	ETH_MEMIF_OWNER_GID_ARG,
 	ETH_MEMIF_MAC_ARG,
 	ETH_MEMIF_ZC_ARG,
 	ETH_MEMIF_SECRET_ARG,
@@ -1515,7 +1519,7 @@ static const struct eth_dev_ops ops = {
 static int
 memif_create(struct rte_vdev_device *vdev, enum memif_role_t role,
 	     memif_interface_id_t id, uint32_t flags,
-	     const char *socket_filename,
+	     const char *socket_filename, uid_t owner_uid, gid_t owner_gid,
 	     memif_log2_ring_size_t log2_ring_size,
 	     uint16_t pkt_buffer_size, const char *secret,
 	     struct rte_ether_addr *ether_addr)
@@ -1554,6 +1558,8 @@ memif_create(struct rte_vdev_device *vdev, enum memif_role_t role,
 	/* Zero-copy flag irelevant to server. */
 	if (pmd->role == MEMIF_ROLE_SERVER)
 		pmd->flags &= ~ETH_MEMIF_FLAG_ZERO_COPY;
+	pmd->owner_uid = owner_uid;
+	pmd->owner_gid = owner_gid;
 
 	ret = memif_socket_init(eth_dev, socket_filename);
 	if (ret < 0)
@@ -1740,6 +1746,30 @@ memif_set_is_socket_abstract(const char *key __rte_unused, const char *value, vo
 	return 0;
 }
 
+static int
+memif_set_owner(const char *key, const char *value, void *extra_args)
+{
+	RTE_ASSERT(sizeof(uid_t) == sizeof(uint32_t));
+	RTE_ASSERT(sizeof(gid_t) == sizeof(uint32_t));
+
+	unsigned long val;
+	char *end = NULL;
+	uint32_t *id = (uint32_t *)extra_args;
+
+	val = strtoul(value, &end, 10);
+	if (*value == '\0' || *end != '\0') {
+		MIF_LOG(ERR, "Failed to parse %s: %s.", key, value);
+		return -EINVAL;
+	}
+	if (val >= UINT32_MAX) {
+		MIF_LOG(ERR, "Invalid %s: %s.", key, value);
+		return -ERANGE;
+	}
+
+	*id = val;
+	return 0;
+}
+
 static int
 memif_set_mac(const char *key __rte_unused, const char *value, void *extra_args)
 {
@@ -1772,6 +1802,8 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 	uint16_t pkt_buffer_size = ETH_MEMIF_DEFAULT_PKT_BUFFER_SIZE;
 	memif_log2_ring_size_t log2_ring_size = ETH_MEMIF_DEFAULT_RING_SIZE;
 	const char *socket_filename = ETH_MEMIF_DEFAULT_SOCKET_FILENAME;
+	uid_t owner_uid = -1;
+	gid_t owner_gid = -1;
 	uint32_t flags = 0;
 	const char *secret = NULL;
 	struct rte_ether_addr *ether_addr = rte_zmalloc("",
@@ -1855,6 +1887,14 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 					 &memif_set_is_socket_abstract, &flags);
 		if (ret < 0)
 			goto exit;
+		ret = rte_kvargs_process(kvlist, ETH_MEMIF_OWNER_UID_ARG,
+					 &memif_set_owner, &owner_uid);
+		if (ret < 0)
+			goto exit;
+		ret = rte_kvargs_process(kvlist, ETH_MEMIF_OWNER_GID_ARG,
+					 &memif_set_owner, &owner_gid);
+		if (ret < 0)
+			goto exit;
 		ret = rte_kvargs_process(kvlist, ETH_MEMIF_MAC_ARG,
 					 &memif_set_mac, ether_addr);
 		if (ret < 0)
@@ -1876,7 +1916,7 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 	}
 
 	/* create interface */
-	ret = memif_create(vdev, role, id, flags, socket_filename,
+	ret = memif_create(vdev, role, id, flags, socket_filename, owner_uid, owner_gid,
 			   log2_ring_size, pkt_buffer_size, secret, ether_addr);
 
 exit:
@@ -1909,7 +1949,9 @@ RTE_PMD_REGISTER_PARAM_STRING(net_memif,
 			      ETH_MEMIF_PKT_BUFFER_SIZE_ARG "=<int>"
 			      ETH_MEMIF_RING_SIZE_ARG "=<int>"
 			      ETH_MEMIF_SOCKET_ARG "=<string>"
-				  ETH_MEMIF_SOCKET_ABSTRACT_ARG "=yes|no"
+			      ETH_MEMIF_SOCKET_ABSTRACT_ARG "=yes|no"
+			      ETH_MEMIF_OWNER_UID_ARG "=<int>"
+			      ETH_MEMIF_OWNER_GID_ARG "=<int>"
 			      ETH_MEMIF_MAC_ARG "=xx:xx:xx:xx:xx:xx"
 			      ETH_MEMIF_ZC_ARG "=yes|no"
 			      ETH_MEMIF_SECRET_ARG "=<string>");
diff --git a/drivers/net/memif/rte_eth_memif.h b/drivers/net/memif/rte_eth_memif.h
index 81e7dceae0..b43895a55f 100644
--- a/drivers/net/memif/rte_eth_memif.h
+++ b/drivers/net/memif/rte_eth_memif.h
@@ -89,6 +89,8 @@ struct pmd_internals {
 /**< use abstract socket address */
 
 	char *socket_filename;			/**< pointer to socket filename */
+	uid_t owner_uid;			/**< socket owner uid */
+	gid_t owner_gid;			/**< socket owner gid */
 	char secret[ETH_MEMIF_SECRET_SIZE]; /**< secret (optional security parameter) */
 
 	struct memif_control_channel *cc;	/**< control channel */
-- 
2.17.1

[PATCH] net/memif: change socket listener owner uid/gid

[Junxiao Shi]

This allows a DPDK application running with root privilege to create a
memif socket listener with non-root owner uid and gid, which can be
connected from client applications running without root privilege.

Signed-off-by: Junxiao Shi <git@mail1.yoursunny.com>
---
 doc/guides/nics/memif.rst         |  2 ++
 drivers/net/memif/memif_socket.c  | 13 +++++++--
 drivers/net/memif/rte_eth_memif.c | 48 +++++++++++++++++++++++++++++--
 drivers/net/memif/rte_eth_memif.h |  2 ++
 4 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/doc/guides/nics/memif.rst b/doc/guides/nics/memif.rst
index aca843640b..8a8141aa72 100644
--- a/doc/guides/nics/memif.rst
+++ b/doc/guides/nics/memif.rst
@@ -44,6 +44,8 @@ client.
    "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 108"
    "socket-abstract=no", "Set usage of abstract socket address", "yes", "yes|no"
+   "uid=1000", "Set socket listener owner uid. Only relevant to server with socket-abstract=no", "unchanged", "uid_t"
+   "gid=1000", "Set socket listener owner gid. Only relevant to server with socket-abstract=no", "unchanged", "gid_t"
    "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 client mode. Only relevant to client, requires '--single-file-segments' eal argument", "no", "yes|no"
diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index 4700ce2e77..34f861afdd 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -889,7 +889,7 @@ memif_listener_handler(void *arg)
 }
 
 static struct memif_socket *
-memif_socket_create(char *key, uint8_t listener, bool is_abstract)
+memif_socket_create(char *key, uint8_t listener, bool is_abstract, uid_t owner_uid, gid_t owner_gid)
 {
 	struct memif_socket *sock;
 	struct sockaddr_un un = { 0 };
@@ -941,6 +941,14 @@ memif_socket_create(char *key, uint8_t listener, bool is_abstract)
 
 		MIF_LOG(DEBUG, "Memif listener socket %s created.", sock->filename);
 
+		if (!is_abstract && (owner_uid != (uid_t)-1 || owner_gid != (gid_t)-1)) {
+			ret = chown(sock->filename, owner_uid, owner_gid);
+			if (ret < 0) {
+				MIF_LOG(ERR, "Failed to change listener socket owner %d", errno);
+				goto error;
+			}
+		}
+
 		/* Allocate interrupt instance */
 		sock->intr_handle =
 			rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_SHARED);
@@ -1017,7 +1025,8 @@ memif_socket_init(struct rte_eth_dev *dev, const char *socket_filename)
 	if (ret < 0) {
 		socket = memif_socket_create(key,
 			(pmd->role == MEMIF_ROLE_CLIENT) ? 0 : 1,
-			pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT);
+			pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT,
+			pmd->owner_uid, pmd->owner_gid);
 		if (socket == NULL)
 			return -1;
 		ret = rte_hash_add_key_data(hash, key, socket);
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index 1b1c1a652b..f82f4bccb8 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -37,6 +37,8 @@
 #define ETH_MEMIF_RING_SIZE_ARG		"rsize"
 #define ETH_MEMIF_SOCKET_ARG		"socket"
 #define ETH_MEMIF_SOCKET_ABSTRACT_ARG	"socket-abstract"
+#define ETH_MEMIF_OWNER_UID_ARG		"owner-uid"
+#define ETH_MEMIF_OWNER_GID_ARG		"owner-gid"
 #define ETH_MEMIF_MAC_ARG		"mac"
 #define ETH_MEMIF_ZC_ARG		"zero-copy"
 #define ETH_MEMIF_SECRET_ARG		"secret"
@@ -48,6 +50,8 @@ static const char * const valid_arguments[] = {
 	ETH_MEMIF_RING_SIZE_ARG,
 	ETH_MEMIF_SOCKET_ARG,
 	ETH_MEMIF_SOCKET_ABSTRACT_ARG,
+	ETH_MEMIF_OWNER_UID_ARG,
+	ETH_MEMIF_OWNER_GID_ARG,
 	ETH_MEMIF_MAC_ARG,
 	ETH_MEMIF_ZC_ARG,
 	ETH_MEMIF_SECRET_ARG,
@@ -1515,7 +1519,7 @@ static const struct eth_dev_ops ops = {
 static int
 memif_create(struct rte_vdev_device *vdev, enum memif_role_t role,
 	     memif_interface_id_t id, uint32_t flags,
-	     const char *socket_filename,
+	     const char *socket_filename, uid_t owner_uid, gid_t owner_gid,
 	     memif_log2_ring_size_t log2_ring_size,
 	     uint16_t pkt_buffer_size, const char *secret,
 	     struct rte_ether_addr *ether_addr)
@@ -1554,6 +1558,8 @@ memif_create(struct rte_vdev_device *vdev, enum memif_role_t role,
 	/* Zero-copy flag irelevant to server. */
 	if (pmd->role == MEMIF_ROLE_SERVER)
 		pmd->flags &= ~ETH_MEMIF_FLAG_ZERO_COPY;
+	pmd->owner_uid = owner_uid;
+	pmd->owner_gid = owner_gid;
 
 	ret = memif_socket_init(eth_dev, socket_filename);
 	if (ret < 0)
@@ -1740,6 +1746,30 @@ memif_set_is_socket_abstract(const char *key __rte_unused, const char *value, vo
 	return 0;
 }
 
+static int
+memif_set_owner(const char *key, const char *value, void *extra_args)
+{
+	RTE_ASSERT(sizeof(uid_t) == sizeof(uint32_t));
+	RTE_ASSERT(sizeof(gid_t) == sizeof(uint32_t));
+
+	unsigned long val;
+	char *end = NULL;
+	uint32_t *id = (uint32_t *)extra_args;
+
+	val = strtoul(value, &end, 10);
+	if (*value == '\0' || *end != '\0') {
+		MIF_LOG(ERR, "Failed to parse %s: %s.", key, value);
+		return -EINVAL;
+	}
+	if (val >= UINT32_MAX) {
+		MIF_LOG(ERR, "Invalid %s: %s.", key, value);
+		return -ERANGE;
+	}
+
+	*id = val;
+	return 0;
+}
+
 static int
 memif_set_mac(const char *key __rte_unused, const char *value, void *extra_args)
 {
@@ -1772,6 +1802,8 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 	uint16_t pkt_buffer_size = ETH_MEMIF_DEFAULT_PKT_BUFFER_SIZE;
 	memif_log2_ring_size_t log2_ring_size = ETH_MEMIF_DEFAULT_RING_SIZE;
 	const char *socket_filename = ETH_MEMIF_DEFAULT_SOCKET_FILENAME;
+	uid_t owner_uid = -1;
+	gid_t owner_gid = -1;
 	uint32_t flags = 0;
 	const char *secret = NULL;
 	struct rte_ether_addr *ether_addr = rte_zmalloc("",
@@ -1855,6 +1887,14 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 					 &memif_set_is_socket_abstract, &flags);
 		if (ret < 0)
 			goto exit;
+		ret = rte_kvargs_process(kvlist, ETH_MEMIF_OWNER_UID_ARG,
+					 &memif_set_owner, &owner_uid);
+		if (ret < 0)
+			goto exit;
+		ret = rte_kvargs_process(kvlist, ETH_MEMIF_OWNER_GID_ARG,
+					 &memif_set_owner, &owner_gid);
+		if (ret < 0)
+			goto exit;
 		ret = rte_kvargs_process(kvlist, ETH_MEMIF_MAC_ARG,
 					 &memif_set_mac, ether_addr);
 		if (ret < 0)
@@ -1876,7 +1916,7 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 	}
 
 	/* create interface */
-	ret = memif_create(vdev, role, id, flags, socket_filename,
+	ret = memif_create(vdev, role, id, flags, socket_filename, owner_uid, owner_gid,
 			   log2_ring_size, pkt_buffer_size, secret, ether_addr);
 
 exit:
@@ -1909,7 +1949,9 @@ RTE_PMD_REGISTER_PARAM_STRING(net_memif,
 			      ETH_MEMIF_PKT_BUFFER_SIZE_ARG "=<int>"
 			      ETH_MEMIF_RING_SIZE_ARG "=<int>"
 			      ETH_MEMIF_SOCKET_ARG "=<string>"
-				  ETH_MEMIF_SOCKET_ABSTRACT_ARG "=yes|no"
+			      ETH_MEMIF_SOCKET_ABSTRACT_ARG "=yes|no"
+			      ETH_MEMIF_OWNER_UID_ARG "=<int>"
+			      ETH_MEMIF_OWNER_GID_ARG "=<int>"
 			      ETH_MEMIF_MAC_ARG "=xx:xx:xx:xx:xx:xx"
 			      ETH_MEMIF_ZC_ARG "=yes|no"
 			      ETH_MEMIF_SECRET_ARG "=<string>");
diff --git a/drivers/net/memif/rte_eth_memif.h b/drivers/net/memif/rte_eth_memif.h
index eb692aee68..6ab7b967a5 100644
--- a/drivers/net/memif/rte_eth_memif.h
+++ b/drivers/net/memif/rte_eth_memif.h
@@ -89,6 +89,8 @@ struct pmd_internals {
 /**< use abstract socket address */
 
 	char *socket_filename;			/**< pointer to socket filename */
+	uid_t owner_uid;			/**< socket owner uid */
+	gid_t owner_gid;			/**< socket owner gid */
 	char secret[ETH_MEMIF_SECRET_SIZE]; /**< secret (optional security parameter) */
 
 	struct memif_control_channel *cc;	/**< control channel */
-- 
2.17.1

Re: [RFC PATCH v3] net/memif: change socket listener owner uid/gid

[Stephen Hemminger]

On Wed, 16 Nov 2022 17:14:13 +0000
Junxiao Shi <git@mail1.yoursunny.com> wrote:

> This allows a DPDK application running with root privilege to create a
> memif socket listener with non-root owner uid and gid, which can be
> connected from client applications running without root privilege.
> 
> Signed-off-by: Junxiao Shi <git@mail1.yoursunny.com>

Looks good, hope you tested this with the example.

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Re: [RFC PATCH v3] net/memif: change socket listener owner uid/gid

[Ferruh Yigit]

On 11/16/2022 5:14 PM, Junxiao Shi wrote:
> This allows a DPDK application running with root privilege to create a
> memif socket listener with non-root owner uid and gid, which can be
> connected from client applications running without root privilege.
> 

+1 to idea

As Stephen mentioned, how this is tested?
Do you have simple memif client that we can use it to verify the work?

> Signed-off-by: Junxiao Shi <git@mail1.yoursunny.com>
> ---
>  doc/guides/nics/memif.rst         |  2 ++
>  drivers/net/memif/memif_socket.c  | 13 +++++++--
>  drivers/net/memif/rte_eth_memif.c | 48 +++++++++++++++++++++++++++++--
>  drivers/net/memif/rte_eth_memif.h |  2 ++
>  4 files changed, 60 insertions(+), 5 deletions(-)
> 
> diff --git a/doc/guides/nics/memif.rst b/doc/guides/nics/memif.rst
> index aca843640b..8a8141aa72 100644
> --- a/doc/guides/nics/memif.rst
> +++ b/doc/guides/nics/memif.rst
> @@ -44,6 +44,8 @@ client.
>     "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 108"
>     "socket-abstract=no", "Set usage of abstract socket address", "yes", "yes|no"
> +   "uid=1000", "Set socket listener owner uid. Only relevant to server with socket-abstract=no", "unchanged", "uid_t"
> +   "gid=1000", "Set socket listener owner gid. Only relevant to server with socket-abstract=no", "unchanged", "gid_t"

The options are "owner-uid" & "owner-gid"

>     "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 client mode. Only relevant to client, requires '--single-file-segments' eal argument", "no", "yes|no"
> diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
> index 7886644412..c2b038d01a 100644
> --- a/drivers/net/memif/memif_socket.c
> +++ b/drivers/net/memif/memif_socket.c
> @@ -889,7 +889,7 @@ memif_listener_handler(void *arg)
>  }
>  
>  static struct memif_socket *
> -memif_socket_create(char *key, uint8_t listener, bool is_abstract)
> +memif_socket_create(char *key, uint8_t listener, bool is_abstract, uid_t owner_uid, gid_t owner_gid)
>  {
>  	struct memif_socket *sock;
>  	struct sockaddr_un un = { 0 };
> @@ -941,6 +941,14 @@ memif_socket_create(char *key, uint8_t listener, bool is_abstract)
>  
>  		MIF_LOG(DEBUG, "Memif listener socket %s created.", sock->filename);
>  
> +		if (!is_abstract && (owner_uid != (uid_t)-1 || owner_gid != (gid_t)-1)) {
> +			ret = chown(sock->filename, owner_uid, owner_gid);
> +			if (ret < 0) {
> +				MIF_LOG(ERR, "Failed to change listener socket owner %d", errno);

When you see the error message it is not clear what is printed '%d' part is.
I can see rest of the driver structured this as:
"<message> : %s, strerror(errno)"
perhaps same can be used here.


> +				goto error;

This path also prints a error log and it also prints 'strerror(errno)',
perhaps 'strerror(errno)' part can be droppped from above log.

<...>

> @@ -1855,6 +1887,14 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
>  					 &memif_set_is_socket_abstract, &flags);
>  		if (ret < 0)
>  			goto exit;
> +		ret = rte_kvargs_process(kvlist, ETH_MEMIF_OWNER_UID_ARG,
> +					 &memif_set_owner, &owner_uid);
> +		if (ret < 0)
> +			goto exit;
> +		ret = rte_kvargs_process(kvlist, ETH_MEMIF_OWNER_GID_ARG,
> +					 &memif_set_owner, &owner_gid);
> +		if (ret < 0)
> +			goto exit;
>  		ret = rte_kvargs_process(kvlist, ETH_MEMIF_MAC_ARG,
>  					 &memif_set_mac, ether_addr);

Unrelated with this patch, but memif checks for valid args and ignores
all silently when at least of them is invalid [1].
Since you are already there, if you have time, can you add a log for the
case when there is invalid argument provided and arguments are ignored?


Thanks,
ferruh


[1]
kvlist = rte_kvargs_parse(rte_vdev_device_args(vdev), valid_arguments);
if (kvlist != NULL) {
	< parse args>
}

Re: [PATCH] net/memif: change socket listener owner uid/gid

[Ferruh Yigit]

On 11/16/2022 5:14 PM, Junxiao Shi wrote:
> This allows a DPDK application running with root privilege to create a
> memif socket listener with non-root owner uid and gid, which can be
> connected from client applications running without root privilege.
> 
> Signed-off-by: Junxiao Shi <git@mail1.yoursunny.com>

Hi Junxiao,

I put some comments to RFC v3, missing this version, can you please
check comments there:
https://patches.dpdk.org/project/dpdk/patch/c079afbbff6669ab@cs.arizona.edu/

[PATCH v2] net/memif: change socket listener owner uid/gid

[Junxiao Shi]

This allows a DPDK application running with root privilege to create a
memif socket listener with non-root owner uid and gid, which can be
connected from client applications running without root privilege.

Signed-off-by: Junxiao Shi <git@mail1.yoursunny.com>
---
 doc/guides/nics/memif.rst         |   2 +
 drivers/net/memif/memif_socket.c  |  13 ++-
 drivers/net/memif/rte_eth_memif.c | 129 ++++++++++++++++++++----------
 drivers/net/memif/rte_eth_memif.h |   2 +
 4 files changed, 102 insertions(+), 44 deletions(-)

diff --git a/doc/guides/nics/memif.rst b/doc/guides/nics/memif.rst
index aca843640b..afc574fdaa 100644
--- a/doc/guides/nics/memif.rst
+++ b/doc/guides/nics/memif.rst
@@ -44,6 +44,8 @@ client.
    "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 108"
    "socket-abstract=no", "Set usage of abstract socket address", "yes", "yes|no"
+   "owner-uid=1000", "Set socket listener owner uid. Only relevant to server with socket-abstract=no", "unchanged", "uid_t"
+   "owner-gid=1000", "Set socket listener owner gid. Only relevant to server with socket-abstract=no", "unchanged", "gid_t"
    "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 client mode. Only relevant to client, requires '--single-file-segments' eal argument", "no", "yes|no"
diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index 4700ce2e77..649f8d0e61 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -889,7 +889,7 @@ memif_listener_handler(void *arg)
 }
 
 static struct memif_socket *
-memif_socket_create(char *key, uint8_t listener, bool is_abstract)
+memif_socket_create(char *key, uint8_t listener, bool is_abstract, uid_t owner_uid, gid_t owner_gid)
 {
 	struct memif_socket *sock;
 	struct sockaddr_un un = { 0 };
@@ -941,6 +941,14 @@ memif_socket_create(char *key, uint8_t listener, bool is_abstract)
 
 		MIF_LOG(DEBUG, "Memif listener socket %s created.", sock->filename);
 
+		if (!is_abstract && (owner_uid != (uid_t)-1 || owner_gid != (gid_t)-1)) {
+			ret = chown(sock->filename, owner_uid, owner_gid);
+			if (ret < 0) {
+				MIF_LOG(ERR, "Failed to change listener socket owner");
+				goto error;
+			}
+		}
+
 		/* Allocate interrupt instance */
 		sock->intr_handle =
 			rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_SHARED);
@@ -1017,7 +1025,8 @@ memif_socket_init(struct rte_eth_dev *dev, const char *socket_filename)
 	if (ret < 0) {
 		socket = memif_socket_create(key,
 			(pmd->role == MEMIF_ROLE_CLIENT) ? 0 : 1,
-			pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT);
+			pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT,
+			pmd->owner_uid, pmd->owner_gid);
 		if (socket == NULL)
 			return -1;
 		ret = rte_hash_add_key_data(hash, key, socket);
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index 1b1c1a652b..871a2bd7d3 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -37,6 +37,8 @@
 #define ETH_MEMIF_RING_SIZE_ARG		"rsize"
 #define ETH_MEMIF_SOCKET_ARG		"socket"
 #define ETH_MEMIF_SOCKET_ABSTRACT_ARG	"socket-abstract"
+#define ETH_MEMIF_OWNER_UID_ARG		"owner-uid"
+#define ETH_MEMIF_OWNER_GID_ARG		"owner-gid"
 #define ETH_MEMIF_MAC_ARG		"mac"
 #define ETH_MEMIF_ZC_ARG		"zero-copy"
 #define ETH_MEMIF_SECRET_ARG		"secret"
@@ -48,6 +50,8 @@ static const char * const valid_arguments[] = {
 	ETH_MEMIF_RING_SIZE_ARG,
 	ETH_MEMIF_SOCKET_ARG,
 	ETH_MEMIF_SOCKET_ABSTRACT_ARG,
+	ETH_MEMIF_OWNER_UID_ARG,
+	ETH_MEMIF_OWNER_GID_ARG,
 	ETH_MEMIF_MAC_ARG,
 	ETH_MEMIF_ZC_ARG,
 	ETH_MEMIF_SECRET_ARG,
@@ -1515,7 +1519,7 @@ static const struct eth_dev_ops ops = {
 static int
 memif_create(struct rte_vdev_device *vdev, enum memif_role_t role,
 	     memif_interface_id_t id, uint32_t flags,
-	     const char *socket_filename,
+	     const char *socket_filename, uid_t owner_uid, gid_t owner_gid,
 	     memif_log2_ring_size_t log2_ring_size,
 	     uint16_t pkt_buffer_size, const char *secret,
 	     struct rte_ether_addr *ether_addr)
@@ -1554,6 +1558,8 @@ memif_create(struct rte_vdev_device *vdev, enum memif_role_t role,
 	/* Zero-copy flag irelevant to server. */
 	if (pmd->role == MEMIF_ROLE_SERVER)
 		pmd->flags &= ~ETH_MEMIF_FLAG_ZERO_COPY;
+	pmd->owner_uid = owner_uid;
+	pmd->owner_gid = owner_gid;
 
 	ret = memif_socket_init(eth_dev, socket_filename);
 	if (ret < 0)
@@ -1740,6 +1746,30 @@ memif_set_is_socket_abstract(const char *key __rte_unused, const char *value, vo
 	return 0;
 }
 
+static int
+memif_set_owner(const char *key, const char *value, void *extra_args)
+{
+	RTE_ASSERT(sizeof(uid_t) == sizeof(uint32_t));
+	RTE_ASSERT(sizeof(gid_t) == sizeof(uint32_t));
+
+	unsigned long val;
+	char *end = NULL;
+	uint32_t *id = (uint32_t *)extra_args;
+
+	val = strtoul(value, &end, 10);
+	if (*value == '\0' || *end != '\0') {
+		MIF_LOG(ERR, "Failed to parse %s: %s.", key, value);
+		return -EINVAL;
+	}
+	if (val >= UINT32_MAX) {
+		MIF_LOG(ERR, "Invalid %s: %s.", key, value);
+		return -ERANGE;
+	}
+
+	*id = val;
+	return 0;
+}
+
 static int
 memif_set_mac(const char *key __rte_unused, const char *value, void *extra_args)
 {
@@ -1772,6 +1802,8 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 	uint16_t pkt_buffer_size = ETH_MEMIF_DEFAULT_PKT_BUFFER_SIZE;
 	memif_log2_ring_size_t log2_ring_size = ETH_MEMIF_DEFAULT_RING_SIZE;
 	const char *socket_filename = ETH_MEMIF_DEFAULT_SOCKET_FILENAME;
+	uid_t owner_uid = -1;
+	gid_t owner_gid = -1;
 	uint32_t flags = 0;
 	const char *secret = NULL;
 	struct rte_ether_addr *ether_addr = rte_zmalloc("",
@@ -1827,47 +1859,58 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 	flags |= ETH_MEMIF_FLAG_SOCKET_ABSTRACT;
 
 	kvlist = rte_kvargs_parse(rte_vdev_device_args(vdev), valid_arguments);
+	if (kvlist == NULL) {
+		MIF_LOG(ERR, "Invalid kvargs key");
+		ret = -EINVAL;
+		goto exit;
+	}
 
 	/* parse parameters */
-	if (kvlist != NULL) {
-		ret = rte_kvargs_process(kvlist, ETH_MEMIF_ROLE_ARG,
-					 &memif_set_role, &role);
-		if (ret < 0)
-			goto exit;
-		ret = rte_kvargs_process(kvlist, ETH_MEMIF_ID_ARG,
-					 &memif_set_id, &id);
-		if (ret < 0)
-			goto exit;
-		ret = rte_kvargs_process(kvlist, ETH_MEMIF_PKT_BUFFER_SIZE_ARG,
-					 &memif_set_bs, &pkt_buffer_size);
-		if (ret < 0)
-			goto exit;
-		ret = rte_kvargs_process(kvlist, ETH_MEMIF_RING_SIZE_ARG,
-					 &memif_set_rs, &log2_ring_size);
-		if (ret < 0)
-			goto exit;
-		ret = rte_kvargs_process(kvlist, ETH_MEMIF_SOCKET_ARG,
-					 &memif_set_socket_filename,
-					 (void *)(&socket_filename));
-		if (ret < 0)
-			goto exit;
-		ret = rte_kvargs_process(kvlist, ETH_MEMIF_SOCKET_ABSTRACT_ARG,
-					 &memif_set_is_socket_abstract, &flags);
-		if (ret < 0)
-			goto exit;
-		ret = rte_kvargs_process(kvlist, ETH_MEMIF_MAC_ARG,
-					 &memif_set_mac, ether_addr);
-		if (ret < 0)
-			goto exit;
-		ret = rte_kvargs_process(kvlist, ETH_MEMIF_ZC_ARG,
-					 &memif_set_zc, &flags);
-		if (ret < 0)
-			goto exit;
-		ret = rte_kvargs_process(kvlist, ETH_MEMIF_SECRET_ARG,
-					 &memif_set_secret, (void *)(&secret));
-		if (ret < 0)
-			goto exit;
-	}
+	ret = rte_kvargs_process(kvlist, ETH_MEMIF_ROLE_ARG,
+					&memif_set_role, &role);
+	if (ret < 0)
+		goto exit;
+	ret = rte_kvargs_process(kvlist, ETH_MEMIF_ID_ARG,
+					&memif_set_id, &id);
+	if (ret < 0)
+		goto exit;
+	ret = rte_kvargs_process(kvlist, ETH_MEMIF_PKT_BUFFER_SIZE_ARG,
+					&memif_set_bs, &pkt_buffer_size);
+	if (ret < 0)
+		goto exit;
+	ret = rte_kvargs_process(kvlist, ETH_MEMIF_RING_SIZE_ARG,
+					&memif_set_rs, &log2_ring_size);
+	if (ret < 0)
+		goto exit;
+	ret = rte_kvargs_process(kvlist, ETH_MEMIF_SOCKET_ARG,
+					&memif_set_socket_filename,
+					(void *)(&socket_filename));
+	if (ret < 0)
+		goto exit;
+	ret = rte_kvargs_process(kvlist, ETH_MEMIF_SOCKET_ABSTRACT_ARG,
+					&memif_set_is_socket_abstract, &flags);
+	if (ret < 0)
+		goto exit;
+	ret = rte_kvargs_process(kvlist, ETH_MEMIF_OWNER_UID_ARG,
+					&memif_set_owner, &owner_uid);
+	if (ret < 0)
+		goto exit;
+	ret = rte_kvargs_process(kvlist, ETH_MEMIF_OWNER_GID_ARG,
+					&memif_set_owner, &owner_gid);
+	if (ret < 0)
+		goto exit;
+	ret = rte_kvargs_process(kvlist, ETH_MEMIF_MAC_ARG,
+					&memif_set_mac, ether_addr);
+	if (ret < 0)
+		goto exit;
+	ret = rte_kvargs_process(kvlist, ETH_MEMIF_ZC_ARG,
+					&memif_set_zc, &flags);
+	if (ret < 0)
+		goto exit;
+	ret = rte_kvargs_process(kvlist, ETH_MEMIF_SECRET_ARG,
+					&memif_set_secret, (void *)(&secret));
+	if (ret < 0)
+		goto exit;
 
 	if (!(flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT)) {
 		ret = memif_check_socket_filename(socket_filename);
@@ -1876,7 +1919,7 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 	}
 
 	/* create interface */
-	ret = memif_create(vdev, role, id, flags, socket_filename,
+	ret = memif_create(vdev, role, id, flags, socket_filename, owner_uid, owner_gid,
 			   log2_ring_size, pkt_buffer_size, secret, ether_addr);
 
 exit:
@@ -1909,7 +1952,9 @@ RTE_PMD_REGISTER_PARAM_STRING(net_memif,
 			      ETH_MEMIF_PKT_BUFFER_SIZE_ARG "=<int>"
 			      ETH_MEMIF_RING_SIZE_ARG "=<int>"
 			      ETH_MEMIF_SOCKET_ARG "=<string>"
-				  ETH_MEMIF_SOCKET_ABSTRACT_ARG "=yes|no"
+			      ETH_MEMIF_SOCKET_ABSTRACT_ARG "=yes|no"
+			      ETH_MEMIF_OWNER_UID_ARG "=<int>"
+			      ETH_MEMIF_OWNER_GID_ARG "=<int>"
 			      ETH_MEMIF_MAC_ARG "=xx:xx:xx:xx:xx:xx"
 			      ETH_MEMIF_ZC_ARG "=yes|no"
 			      ETH_MEMIF_SECRET_ARG "=<string>");
diff --git a/drivers/net/memif/rte_eth_memif.h b/drivers/net/memif/rte_eth_memif.h
index eb692aee68..6ab7b967a5 100644
--- a/drivers/net/memif/rte_eth_memif.h
+++ b/drivers/net/memif/rte_eth_memif.h
@@ -89,6 +89,8 @@ struct pmd_internals {
 /**< use abstract socket address */
 
 	char *socket_filename;			/**< pointer to socket filename */
+	uid_t owner_uid;			/**< socket owner uid */
+	gid_t owner_gid;			/**< socket owner gid */
 	char secret[ETH_MEMIF_SECRET_SIZE]; /**< secret (optional security parameter) */
 
 	struct memif_control_channel *cc;	/**< control channel */
-- 
2.17.1

Re: [PATCH v2] net/memif: change socket listener owner uid/gid

[Ferruh Yigit]

On 12/7/2022 2:41 PM, Junxiao Shi wrote:
> This allows a DPDK application running with root privilege to create a
> memif socket listener with non-root owner uid and gid, which can be
> connected from client applications running without root privilege.
> 

Do you have an easy way to test unprivileged memif client?

> Signed-off-by: Junxiao Shi <git@mail1.yoursunny.com>

<...>

> @@ -1827,47 +1859,58 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
>  	flags |= ETH_MEMIF_FLAG_SOCKET_ABSTRACT;
>  
>  	kvlist = rte_kvargs_parse(rte_vdev_device_args(vdev), valid_arguments);
> +	if (kvlist == NULL) {
> +		MIF_LOG(ERR, "Invalid kvargs key");
> +		ret = -EINVAL;
> +		goto exit;
> +	}

Thanks Junxiao for updating this, but since it is not really related to
this patch, can you please separate it to another patch?

[PATCH v3] net/memif: change socket listener owner uid/gid

[Junxiao Shi]

This allows a DPDK application running with root privilege to create a
memif socket listener with non-root owner uid and gid, which can be
connected from client applications running without root privilege.

Signed-off-by: Junxiao Shi <git@mail1.yoursunny.com>
---
 doc/guides/nics/memif.rst         |  2 ++
 drivers/net/memif/memif_socket.c  | 13 +++++++--
 drivers/net/memif/rte_eth_memif.c | 48 +++++++++++++++++++++++++++++--
 drivers/net/memif/rte_eth_memif.h |  2 ++
 4 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/doc/guides/nics/memif.rst b/doc/guides/nics/memif.rst
index aca843640b..afc574fdaa 100644
--- a/doc/guides/nics/memif.rst
+++ b/doc/guides/nics/memif.rst
@@ -44,6 +44,8 @@ client.
    "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 108"
    "socket-abstract=no", "Set usage of abstract socket address", "yes", "yes|no"
+   "owner-uid=1000", "Set socket listener owner uid. Only relevant to server with socket-abstract=no", "unchanged", "uid_t"
+   "owner-gid=1000", "Set socket listener owner gid. Only relevant to server with socket-abstract=no", "unchanged", "gid_t"
    "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 client mode. Only relevant to client, requires '--single-file-segments' eal argument", "no", "yes|no"
diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index 4700ce2e77..649f8d0e61 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -889,7 +889,7 @@ memif_listener_handler(void *arg)
 }
 
 static struct memif_socket *
-memif_socket_create(char *key, uint8_t listener, bool is_abstract)
+memif_socket_create(char *key, uint8_t listener, bool is_abstract, uid_t owner_uid, gid_t owner_gid)
 {
 	struct memif_socket *sock;
 	struct sockaddr_un un = { 0 };
@@ -941,6 +941,14 @@ memif_socket_create(char *key, uint8_t listener, bool is_abstract)
 
 		MIF_LOG(DEBUG, "Memif listener socket %s created.", sock->filename);
 
+		if (!is_abstract && (owner_uid != (uid_t)-1 || owner_gid != (gid_t)-1)) {
+			ret = chown(sock->filename, owner_uid, owner_gid);
+			if (ret < 0) {
+				MIF_LOG(ERR, "Failed to change listener socket owner");
+				goto error;
+			}
+		}
+
 		/* Allocate interrupt instance */
 		sock->intr_handle =
 			rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_SHARED);
@@ -1017,7 +1025,8 @@ memif_socket_init(struct rte_eth_dev *dev, const char *socket_filename)
 	if (ret < 0) {
 		socket = memif_socket_create(key,
 			(pmd->role == MEMIF_ROLE_CLIENT) ? 0 : 1,
-			pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT);
+			pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT,
+			pmd->owner_uid, pmd->owner_gid);
 		if (socket == NULL)
 			return -1;
 		ret = rte_hash_add_key_data(hash, key, socket);
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index 1b1c1a652b..f82f4bccb8 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -37,6 +37,8 @@
 #define ETH_MEMIF_RING_SIZE_ARG		"rsize"
 #define ETH_MEMIF_SOCKET_ARG		"socket"
 #define ETH_MEMIF_SOCKET_ABSTRACT_ARG	"socket-abstract"
+#define ETH_MEMIF_OWNER_UID_ARG		"owner-uid"
+#define ETH_MEMIF_OWNER_GID_ARG		"owner-gid"
 #define ETH_MEMIF_MAC_ARG		"mac"
 #define ETH_MEMIF_ZC_ARG		"zero-copy"
 #define ETH_MEMIF_SECRET_ARG		"secret"
@@ -48,6 +50,8 @@ static const char * const valid_arguments[] = {
 	ETH_MEMIF_RING_SIZE_ARG,
 	ETH_MEMIF_SOCKET_ARG,
 	ETH_MEMIF_SOCKET_ABSTRACT_ARG,
+	ETH_MEMIF_OWNER_UID_ARG,
+	ETH_MEMIF_OWNER_GID_ARG,
 	ETH_MEMIF_MAC_ARG,
 	ETH_MEMIF_ZC_ARG,
 	ETH_MEMIF_SECRET_ARG,
@@ -1515,7 +1519,7 @@ static const struct eth_dev_ops ops = {
 static int
 memif_create(struct rte_vdev_device *vdev, enum memif_role_t role,
 	     memif_interface_id_t id, uint32_t flags,
-	     const char *socket_filename,
+	     const char *socket_filename, uid_t owner_uid, gid_t owner_gid,
 	     memif_log2_ring_size_t log2_ring_size,
 	     uint16_t pkt_buffer_size, const char *secret,
 	     struct rte_ether_addr *ether_addr)
@@ -1554,6 +1558,8 @@ memif_create(struct rte_vdev_device *vdev, enum memif_role_t role,
 	/* Zero-copy flag irelevant to server. */
 	if (pmd->role == MEMIF_ROLE_SERVER)
 		pmd->flags &= ~ETH_MEMIF_FLAG_ZERO_COPY;
+	pmd->owner_uid = owner_uid;
+	pmd->owner_gid = owner_gid;
 
 	ret = memif_socket_init(eth_dev, socket_filename);
 	if (ret < 0)
@@ -1740,6 +1746,30 @@ memif_set_is_socket_abstract(const char *key __rte_unused, const char *value, vo
 	return 0;
 }
 
+static int
+memif_set_owner(const char *key, const char *value, void *extra_args)
+{
+	RTE_ASSERT(sizeof(uid_t) == sizeof(uint32_t));
+	RTE_ASSERT(sizeof(gid_t) == sizeof(uint32_t));
+
+	unsigned long val;
+	char *end = NULL;
+	uint32_t *id = (uint32_t *)extra_args;
+
+	val = strtoul(value, &end, 10);
+	if (*value == '\0' || *end != '\0') {
+		MIF_LOG(ERR, "Failed to parse %s: %s.", key, value);
+		return -EINVAL;
+	}
+	if (val >= UINT32_MAX) {
+		MIF_LOG(ERR, "Invalid %s: %s.", key, value);
+		return -ERANGE;
+	}
+
+	*id = val;
+	return 0;
+}
+
 static int
 memif_set_mac(const char *key __rte_unused, const char *value, void *extra_args)
 {
@@ -1772,6 +1802,8 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 	uint16_t pkt_buffer_size = ETH_MEMIF_DEFAULT_PKT_BUFFER_SIZE;
 	memif_log2_ring_size_t log2_ring_size = ETH_MEMIF_DEFAULT_RING_SIZE;
 	const char *socket_filename = ETH_MEMIF_DEFAULT_SOCKET_FILENAME;
+	uid_t owner_uid = -1;
+	gid_t owner_gid = -1;
 	uint32_t flags = 0;
 	const char *secret = NULL;
 	struct rte_ether_addr *ether_addr = rte_zmalloc("",
@@ -1855,6 +1887,14 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 					 &memif_set_is_socket_abstract, &flags);
 		if (ret < 0)
 			goto exit;
+		ret = rte_kvargs_process(kvlist, ETH_MEMIF_OWNER_UID_ARG,
+					 &memif_set_owner, &owner_uid);
+		if (ret < 0)
+			goto exit;
+		ret = rte_kvargs_process(kvlist, ETH_MEMIF_OWNER_GID_ARG,
+					 &memif_set_owner, &owner_gid);
+		if (ret < 0)
+			goto exit;
 		ret = rte_kvargs_process(kvlist, ETH_MEMIF_MAC_ARG,
 					 &memif_set_mac, ether_addr);
 		if (ret < 0)
@@ -1876,7 +1916,7 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 	}
 
 	/* create interface */
-	ret = memif_create(vdev, role, id, flags, socket_filename,
+	ret = memif_create(vdev, role, id, flags, socket_filename, owner_uid, owner_gid,
 			   log2_ring_size, pkt_buffer_size, secret, ether_addr);
 
 exit:
@@ -1909,7 +1949,9 @@ RTE_PMD_REGISTER_PARAM_STRING(net_memif,
 			      ETH_MEMIF_PKT_BUFFER_SIZE_ARG "=<int>"
 			      ETH_MEMIF_RING_SIZE_ARG "=<int>"
 			      ETH_MEMIF_SOCKET_ARG "=<string>"
-				  ETH_MEMIF_SOCKET_ABSTRACT_ARG "=yes|no"
+			      ETH_MEMIF_SOCKET_ABSTRACT_ARG "=yes|no"
+			      ETH_MEMIF_OWNER_UID_ARG "=<int>"
+			      ETH_MEMIF_OWNER_GID_ARG "=<int>"
 			      ETH_MEMIF_MAC_ARG "=xx:xx:xx:xx:xx:xx"
 			      ETH_MEMIF_ZC_ARG "=yes|no"
 			      ETH_MEMIF_SECRET_ARG "=<string>");
diff --git a/drivers/net/memif/rte_eth_memif.h b/drivers/net/memif/rte_eth_memif.h
index eb692aee68..6ab7b967a5 100644
--- a/drivers/net/memif/rte_eth_memif.h
+++ b/drivers/net/memif/rte_eth_memif.h
@@ -89,6 +89,8 @@ struct pmd_internals {
 /**< use abstract socket address */
 
 	char *socket_filename;			/**< pointer to socket filename */
+	uid_t owner_uid;			/**< socket owner uid */
+	gid_t owner_gid;			/**< socket owner gid */
 	char secret[ETH_MEMIF_SECRET_SIZE]; /**< secret (optional security parameter) */
 
 	struct memif_control_channel *cc;	/**< control channel */
-- 
2.17.1

Re: [PATCH v2] net/memif: change socket listener owner uid/gid

[Ferruh Yigit]

On 12/7/2022 3:43 PM, Ferruh Yigit wrote:
> On 12/7/2022 2:41 PM, Junxiao Shi wrote:
>> This allows a DPDK application running with root privilege to create a
>> memif socket listener with non-root owner uid and gid, which can be
>> connected from client applications running without root privilege.
>>
> 
> Do you have an easy way to test unprivileged memif client?
> 
>> Signed-off-by: Junxiao Shi <git@mail1.yoursunny.com>
> 
> <...>
> 
>> @@ -1827,47 +1859,58 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
>>  	flags |= ETH_MEMIF_FLAG_SOCKET_ABSTRACT;
>>  
>>  	kvlist = rte_kvargs_parse(rte_vdev_device_args(vdev), valid_arguments);
>> +	if (kvlist == NULL) {
>> +		MIF_LOG(ERR, "Invalid kvargs key");
>> +		ret = -EINVAL;
>> +		goto exit;
>> +	}
> 
> Thanks Junxiao for updating this, but since it is not really related to
> this patch, can you please separate it to another patch?

Also I am not sure what 'rte_kvargs_parse()' returns if there is no
argument provided (that may be the case if all default values are desired).
If API returns NULL for that case, above error log can be wrong, can you
please check.

Re: [PATCH v2] net/memif: change socket listener owner uid/gid

[Junxiao Shi]

[-- Attachment #1: Type: text/plain, Size: 1839 bytes --]

Hi Ferruh

> On 12/7/2022 2:41 PM, Junxiao Shi wrote:
> > This allows a DPDK application running with root privilege to create a
> > memif socket listener with non-root owner uid and gid, which can be
> > connected from client applications running without root privilege.
> >
>
> Do you have an easy way to test unprivileged memif client?

This has been tested with NDN-DPDK software.
https://github.com/usnistgov/ndn-dpdk revision
311de078aa4dc3ea28db5f8858e70a1bef7b9ccd

The systemd service is running as root and it uses DPDK with the owner-uid
and owner-gid args.
The ndndpdk-godemo command is running as unprivileged process.
Directory /run/ndn still needs to be created by root.

These commands can perform a full test:

git clone https://github.com/usnistgov/ndn-dpdk.git
cd ndn-dpdk
./docs/ndndpdk-depends.sh --dpdk-patch=26031
corepack pnpm install
make
sudo make install
sudo dpdk-hugepages.py --setup 8G
sudo ndndpdk-ctrl systemd start
jq -n {} | ndndpdk-ctrl activate-forwarder
sudo mkdir -p /run/ndn
ndndpdk-godemo pingserver --name /A
ndndpdk-godemo pingclient --name /A

You can see packets flowing through.
Run `ls -l /run/ndn` and check the uid:gid of socket files too.


>
> > Signed-off-by: Junxiao Shi <git@mail1.yoursunny.com>
>
> <...>
>
> > @@ -1827,47 +1859,58 @@ rte_pmd_memif_probe(struct rte_vdev_device
*vdev)
> >       flags |= ETH_MEMIF_FLAG_SOCKET_ABSTRACT;
> >
> >       kvlist = rte_kvargs_parse(rte_vdev_device_args(vdev),
valid_arguments);
> > +     if (kvlist == NULL) {
> > +             MIF_LOG(ERR, "Invalid kvargs key");
> > +             ret = -EINVAL;
> > +             goto exit;
> > +     }
>
> Thanks Junxiao for updating this, but since it is not really related to
> this patch, can you please separate it to another patch?

These are reverted and will be submitted separately in the future.

[-- Attachment #2: Type: text/html, Size: 2581 bytes --]

Re: [PATCH v2] net/memif: change socket listener owner uid/gid

[Ferruh Yigit]

On 12/7/2022 5:48 PM, Junxiao Shi wrote:
> Hi Ferruh
> 
>> On 12/7/2022 2:41 PM, Junxiao Shi wrote:
>> > This allows a DPDK application running with root privilege to create a
>> > memif socket listener with non-root owner uid and gid, which can be
>> > connected from client applications running without root privilege.
>> >
>>
>> Do you have an easy way to test unprivileged memif client?
> 
> This has been tested with NDN-DPDK software.
> https://github.com/usnistgov/ndn-dpdk
> <https://github.com/usnistgov/ndn-dpdk> revision
> 311de078aa4dc3ea28db5f8858e70a1bef7b9ccd
> 

Thanks for the info.

Do you want this project to be included in DPDK web page [1], if so you
can request this in web mail list (web@dpdk.org).

[1]
https://www.dpdk.org/ecosystem/#projects


> The systemd service is running as root and it uses DPDK with the
> owner-uid and owner-gid args.
> The ndndpdk-godemo command is running as unprivileged process.
> Directory /run/ndn still needs to be created by root.
> 
> These commands can perform a full test:
> 
> git clone https://github.com/usnistgov/ndn-dpdk.git
> <https://github.com/usnistgov/ndn-dpdk.git>
> cd ndn-dpdk
> ./docs/ndndpdk-depends.sh --dpdk-patch=26031
> corepack pnpm install
> make
> sudo make install
> sudo dpdk-hugepages.py --setup 8G
> sudo ndndpdk-ctrl systemd start
> jq -n {} | ndndpdk-ctrl activate-forwarder
> sudo mkdir -p /run/ndn
> ndndpdk-godemo pingserver --name /A
> ndndpdk-godemo pingclient --name /A
> 
> You can see packets flowing through.
> Run `ls -l /run/ndn` and check the uid:gid of socket files too.
> 

It is good to record these steps, but for now I will rely on your
testing :), thanks.

> 
>>
>> > Signed-off-by: Junxiao Shi <git@mail1.yoursunny.com
> <mailto:git@mail1.yoursunny.com>>
>>
>> <...>
>>
>> > @@ -1827,47 +1859,58 @@ rte_pmd_memif_probe(struct rte_vdev_device
> *vdev)
>> >       flags |= ETH_MEMIF_FLAG_SOCKET_ABSTRACT;
>> >
>> >       kvlist = rte_kvargs_parse(rte_vdev_device_args(vdev),
> valid_arguments);
>> > +     if (kvlist == NULL) {
>> > +             MIF_LOG(ERR, "Invalid kvargs key");
>> > +             ret = -EINVAL;
>> > +             goto exit;
>> > +     }
>>
>> Thanks Junxiao for updating this, but since it is not really related to
>> this patch, can you please separate it to another patch?
> 
> These are reverted and will be submitted separately in the future.

ack

Re: [PATCH v3] net/memif: change socket listener owner uid/gid

[Ferruh Yigit]

On 12/7/2022 3:53 PM, Junxiao Shi wrote:
> This allows a DPDK application running with root privilege to create a
> memif socket listener with non-root owner uid and gid, which can be
> connected from client applications running without root privilege.
> 
> Signed-off-by: Junxiao Shi <git@mail1.yoursunny.com>

(moving from older version)
Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>

Applied to dpdk-next-net/main, thanks.

Re: [PATCH v3] net/memif: change socket listener owner uid/gid

[Ferruh Yigit]

On 12/7/2022 3:53 PM, Junxiao Shi wrote:
> This allows a DPDK application running with root privilege to create a
> memif socket listener with non-root owner uid and gid, which can be
> connected from client applications running without root privilege.
> 
> Signed-off-by: Junxiao Shi <git@mail1.yoursunny.com>

(moving from older version)
Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>

Applied to dpdk-next-net/main, thanks.