Re: [dpdk-dev] [RFC v3] net/memif: allow for full key size in socket name

["Yigit, Ferruh" <ferruh.yigit@linux.intel.com>]

On 7/16/2019 6:20 PM, Stephen Hemminger wrote:
> The key size for memif is 256 but the unix domain socket structure has
> space for 100 bytes. Change it to use a larger buffer and not hard
> code the keysize everywhere.
> 
> Not sure what purpose of socket is anyway since there is no code
> which connects to it in the current tree anyway?
> 
> Still an RFC, have no way to test.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> v3 - fix checkpatch issues
> 
>  drivers/net/memif/memif_socket.c | 29 ++++++++++++++++++-----------
>  drivers/net/memif/memif_socket.h |  4 +++-
>  2 files changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
> index 01a935f87c9f..5eecbdc98040 100644
> --- a/drivers/net/memif/memif_socket.c
> +++ b/drivers/net/memif/memif_socket.c
> @@ -860,11 +860,16 @@ memif_listener_handler(void *arg)
>  		rte_free(cc);
>  }
>  
> +#define MEMIF_SOCKET_UN_SIZE	\
> +	(offsetof(struct sockaddr_un, sun_path) + MEMIF_SOCKET_KEY_LEN)
> +
>  static struct memif_socket *
> -memif_socket_create(struct pmd_internals *pmd, char *key, uint8_t listener)
> +memif_socket_create(struct pmd_internals *pmd,
> +		    const char *key, uint8_t listener)
>  {
>  	struct memif_socket *sock;
> -	struct sockaddr_un un;
> +	struct sockaddr_un *un;
> +	char un_buf[MEMIF_SOCKET_UN_SIZE];
>  	int sockfd;
>  	int ret;
>  	int on = 1;
> @@ -876,7 +881,7 @@ memif_socket_create(struct pmd_internals *pmd, char *key, uint8_t listener)
>  	}
>  
>  	sock->listener = listener;
> -	rte_memcpy(sock->filename, key, 256);
> +	strlcpy(sock->filename, key, MEMIF_SOCKET_KEY_LEN);
>  	TAILQ_INIT(&sock->dev_queue);
>  
>  	if (listener != 0) {
> @@ -884,15 +889,16 @@ memif_socket_create(struct pmd_internals *pmd, char *key, uint8_t listener)
>  		if (sockfd < 0)
>  			goto error;
>  
> -		un.sun_family = AF_UNIX;
> -		memcpy(un.sun_path, sock->filename,
> -			sizeof(un.sun_path) - 1);
> +		memset(un_buf, 0, sizeof(un_buf));
> +		un = (struct sockaddr_un *)un_buf;
> +		un->sun_family = AF_UNIX;
> +		strlcpy(un->sun_path, sock->filename, MEMIF_SOCKET_KEY_LEN);
>  
>  		ret = setsockopt(sockfd, SOL_SOCKET, SO_PASSCRED, &on,
>  				 sizeof(on));
>  		if (ret < 0)
>  			goto error;
> -		ret = bind(sockfd, (struct sockaddr *)&un, sizeof(un));
> +		ret = bind(sockfd, (struct sockaddr *)un, MEMIF_SOCKET_UN_SIZE);

Hi Jakub,

While testing your zero-copy patch [1], I stuck to a bind() error [2].
When provided a socket length bigger than "sizeof(struct sockaddr)", bind()
fails. I am testing this on a Fedora system.
I wonder if there is a check in glibc related to the length.

What was your test platform for the change?



[1]
https://patches.dpdk.org/patch/57817/

[2]
memif_socket_create(): NULL: Failed to setup socket /run/memif.sock: Invalid
argument