net/memif: change socket listener owner uid/gid
RFC PATCH 1/1 net/memif: change socket listener owner uid/gid
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.
--- 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(-)
doc/guides/nics/memif.rst +2 -0
@@ -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"
drivers/net/memif/memif_socket.c +11 -2
@@ -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 Ð_MEMIF_FLAG_SOCKET_ABSTRACT ); + pmd->flags Ð_MEMIF_FLAG_SOCKET_ABSTRACT , + pmd->owner_uid, pmd->owner_gid);if (socket ==NULL )return -1 ; ret =rte_hash_add_key_data (hash, key, socket);
drivers/net/memif/rte_eth_memif.c +45 -11
109 lines not on this page · raw message
drivers/net/memif/rte_eth_memif.h +2 -0
@@ -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 */
Signature · 1 line
2.17.1
Quoted 6 lines · On Tue, 15 Nov 2022 20:44:41 +0000 Junxiao Shi <[email protected]> wrote:
drivers/net/memif/rte_eth_memif.c +6
-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 ;
drivers/net/memif/rte_eth_memif.c memif_set_is_socket_abstract(const char *key __rte_unused, const char *value, vo · line 1764
Quoted 10 lines
drivers/net/memif/rte_eth_memif.c +10
-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 1/1 net/memif: change socket listener owner uid/gid
In reply to Junxiao ShiThis 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.
--- 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(-)
doc/guides/nics/memif.rst +2 -0
@@ -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"
drivers/net/memif/memif_socket.c +11 -2
@@ -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 Ð_MEMIF_FLAG_SOCKET_ABSTRACT ); + pmd->flags Ð_MEMIF_FLAG_SOCKET_ABSTRACT , + pmd->owner_uid, pmd->owner_gid);if (socket ==NULL )return -1 ; ret =rte_hash_add_key_data (hash, key, socket);
drivers/net/memif/rte_eth_memif.c +43 -3
100 lines not on this page · raw message
drivers/net/memif/rte_eth_memif.h +2 -0
@@ -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 */
Signature · 1 line
2.17.1
drivers/net/memif/rte_eth_memif.c memif_set_is_socket_abstract(const char *key __rte_unused, const char *value, vo · line 1753
Quoted 5 lines · On Wed, 16 Nov 2022 13:09:20 +0000 Junxiao Shi <[email protected]> wrote:
drivers/net/memif/rte_eth_memif.c +5
-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 ),"" );
Use RTE_ASSERT() for these
drivers/net/memif/rte_eth_memif.c memif_set_is_socket_abstract(const char *key __rte_unused, const char *value, vo · line 1757
Quoted 4 lines
drivers/net/memif/rte_eth_memif.c +4
-0+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 1/1 net/memif: change socket listener owner uid/gid
In reply to Junxiao ShiThis 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.
--- 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(-)
doc/guides/nics/memif.rst +2 -0
@@ -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"
drivers/net/memif/memif_socket.c +11 -2
@@ -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 Ð_MEMIF_FLAG_SOCKET_ABSTRACT ); + pmd->flags Ð_MEMIF_FLAG_SOCKET_ABSTRACT , + pmd->owner_uid, pmd->owner_gid);if (socket ==NULL )return -1 ; ret =rte_hash_add_key_data (hash, key, socket);
drivers/net/memif/rte_eth_memif.c +45 -3
102 lines not on this page · raw message
drivers/net/memif/rte_eth_memif.h +2 -0
@@ -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 */
Signature · 1 line
2.17.1
Quoted 5 lines · On Wed, 16 Nov 2022 17:14:13 +0000 Junxiao Shi <[email protected]> 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 <[email protected]>
Looks good, hope you tested this with the example.
PATCH 1/1 net/memif: change socket listener owner uid/gid
In reply to Junxiao ShiThis 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.
--- 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(-)
doc/guides/nics/memif.rst +2 -0
@@ -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"
drivers/net/memif/memif_socket.c +11 -2
@@ -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 Ð_MEMIF_FLAG_SOCKET_ABSTRACT ); + pmd->flags Ð_MEMIF_FLAG_SOCKET_ABSTRACT , + pmd->owner_uid, pmd->owner_gid);if (socket ==NULL )return -1 ; ret =rte_hash_add_key_data (hash, key, socket);
drivers/net/memif/rte_eth_memif.c +45 -3
102 lines not on this page · raw message
drivers/net/memif/rte_eth_memif.h +2 -0
@@ -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 */
Signature · 1 line
2.17.1
Quoted 5 lines · 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 <[email protected]>
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/[email protected]/
PATCH v2 1/1 net/memif: change socket listener owner uid/gid
In reply to Junxiao ShiThis 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.
--- 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(-)
doc/guides/nics/memif.rst +2 -0
@@ -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"
drivers/net/memif/memif_socket.c +11 -2
@@ -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 Ð_MEMIF_FLAG_SOCKET_ABSTRACT ); + pmd->flags Ð_MEMIF_FLAG_SOCKET_ABSTRACT , + pmd->owner_uid, pmd->owner_gid);if (socket ==NULL )return -1 ; ret =rte_hash_add_key_data (hash, key, socket);
drivers/net/memif/rte_eth_memif.c +87 -42
185 lines not on this page · raw message
drivers/net/memif/rte_eth_memif.h +2 -0
@@ -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 */
Signature · 1 line
2.17.1
Quoted 4 lines · 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.
Do you have an easy way to test unprivileged memif client?
Quoted 1 line
Signed-off-by Junxiao Shi <[email protected]>
<...>
drivers/net/memif/rte_eth_memif.c rte_pmd_memif_probe(struct rte_vdev_device *vdev) · line 1866
Quoted 9 lines
drivers/net/memif/rte_eth_memif.c +5
-0@@ -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?
Quoted 15 lines · Ferruh Yigit
Quoted 4 lines · 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.
Do you have an easy way to test unprivileged memif client?
Quoted 1 line
Signed-off-by Junxiao Shi <[email protected]><...>
Quoted 3 lines
drivers/net/memif/rte_eth_memif.c +0
-0@@ -1827,47 +1859,58 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev) flags |=ETH_MEMIF_FLAG_SOCKET_ABSTRACT ;
Quoted 9 lines
drivers/net/memif/rte_eth_memif.c rte_pmd_memif_probe(struct rte_vdev_device *vdev) · line 1866
Quoted 6 lines
drivers/net/memif/rte_eth_memif.c +5
-0kvlist =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.
Hi Ferruh
Quoted 7 lines · Junxiao Shi
Quoted 4 lines
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 revision311de078aa4dc3ea28db5f8858e70a1bef7b9ccd
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.
Quoted 6 lines
Quoted 2 lines
Signed-off-by Junxiao Shi <[email protected]><...>
Review rte_pmd_memif_probe(struct rte_vdev_device
Quoted 1 line
(quoted diff) +0
-0@@ -1827,47 +1859,58 @@ rte_pmd_memif_probe(struct rte_vdev_device
*vdev)
Quoted 3 lines
drivers/net/memif/rte_eth_memif.c rte_pmd_memif_probe(struct rte_vdev_device *vdev)
Quoted 3 lines
drivers/net/memif/rte_eth_memif.c +0
-0flags |=ETH_MEMIF_FLAG_SOCKET_ABSTRACT ; kvlist = rte_kvargs_parse(rte_vdev_device_args(vdev),
valid_arguments);
Quoted 8 lines
drivers/net/memif/rte_eth_memif.c rte_pmd_memif_probe(struct rte_vdev_device *vdev) · line 1866
Quoted 5 lines
drivers/net/memif/rte_eth_memif.c +5
-0+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.
Quoted 8 lines · Junxiao Shi
Hi Ferruh
Quoted 6 lines · Junxiao Shi
Quoted 4 lines
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.
Quoted 7 lines
Quoted 1 line
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> revision311de078aa4dc3ea28db5f8858e70a1bef7b9ccd
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 ([email protected]).
[1]
https://www.dpdk.org/ecosystem/#projects
Quoted 21 lines
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/ndnstill 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
sudodpdk-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
Quoted 3 lines
You can see packets flowing through.
Runls -l /run/ndnand 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.
Quoted 19 lines
Quoted 3 lines
Quoted 3 lines
Signed-off-by Junxiao Shi <[email protected]<mailto:[email protected]>>
Quoted 4 lines
<...>
Review rte_pmd_memif_probe(struct rte_vdev_device
Quoted 1 line
(quoted diff) +0
-0@@ -1827,47 +1859,58 @@ rte_pmd_memif_probe(struct rte_vdev_device*vdev)
Quoted 3 lines
drivers/net/memif/rte_eth_memif.c rte_pmd_memif_probe(struct rte_vdev_device *vdev)
Quoted 3 lines
drivers/net/memif/rte_eth_memif.c +0
-0flags |= ETH_MEMIF_FLAG_SOCKET_ABSTRACT; kvlist = rte_kvargs_parse(rte_vdev_device_args(vdev),
valid_arguments);Quoted 6 lines
Quoted 5 lines
drivers/net/memif/rte_eth_memif.c +5
-0+ if (kvlist == NULL) { + MIF_LOG(ERR, "Invalid kvargs key"); + ret = -EINVAL; + goto exit; + }
Quoted 4 lines
Quoted 2 lines
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
PATCH v3 1/1 net/memif: change socket listener owner uid/gid
In reply to Junxiao ShiThis 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.
--- 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(-)
doc/guides/nics/memif.rst +2 -0
@@ -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"
drivers/net/memif/memif_socket.c +11 -2
@@ -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 Ð_MEMIF_FLAG_SOCKET_ABSTRACT ); + pmd->flags Ð_MEMIF_FLAG_SOCKET_ABSTRACT , + pmd->owner_uid, pmd->owner_gid);if (socket ==NULL )return -1 ; ret =rte_hash_add_key_data (hash, key, socket);
drivers/net/memif/rte_eth_memif.c +45 -3
102 lines not on this page · raw message
drivers/net/memif/rte_eth_memif.h +2 -0
@@ -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 */
Signature · 1 line
2.17.1
Quoted 5 lines · 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 <[email protected]>
(moving from older version)
Applied to dpdk-next-net/main, thanks.
Quoted 5 lines · 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 <[email protected]>
(moving from older version)
Applied to dpdk-next-net/main, thanks.
Quoted 4 lines · 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.
+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?
Quoted 13 lines
Signed-off-by Junxiao Shi <[email protected]>--- 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(-)doc/guides/nics/memif.rst +0
-0
doc/guides/nics/memif.rst client. · line 48
Quoted 5 lines
doc/guides/nics/memif.rst +2
-0@@ -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"
Quoted 17 lines
doc/guides/nics/memif.rst +0
-0"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"drivers/net/memif/memif_socket.c +1
-1@@ -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 };
drivers/net/memif/memif_socket.c memif_socket_create(char *key, uint8_t listener, bool is_abstract) · line 947
Quoted 7 lines
drivers/net/memif/memif_socket.c +4
-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.
drivers/net/memif/memif_socket.c memif_socket_create(char *key, uint8_t listener, bool is_abstract) · line 948
Quoted 1 line
drivers/net/memif/memif_socket.c +1
-0+ 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.
<...>
Quoted 4 lines
drivers/net/memif/memif_socket.c +0
-0@@ -1855,6 +1887,14 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev) &memif_set_is_socket_abstract, &flags);if (ret <0 ) goto exit;
drivers/net/memif/memif_socket.c rte_pmd_memif_probe(struct rte_vdev_device *vdev) · line 1899
Quoted 10 lines
drivers/net/memif/memif_socket.c +8
-0+ 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>
}