Re: [PATCH for-9.1 v3 05/11] contrib/vhost-user-blk: fix bind() using the right size of the address

2024-04-04 Thread Philippe Mathieu-Daudé 

On 4/4/24 14:23, Stefano Garzarella wrote:

On macOS passing `-s /tmp/vhost.socket` parameter to the vhost-user-blk
application, the bind was done on `/tmp/vhost.socke` pathname,
missing the last character.

This sounds like one of the portability problems described in the
unix(7) manpage:

Pathname sockets
When  binding  a socket to a pathname, a few rules should
be observed for maximum portability and ease of coding:

•  The pathname in sun_path should be null-terminated.

•  The length of the pathname, including the  terminating
   null byte, should not exceed the size of sun_path.

•  The  addrlen  argument  that  describes  the enclosing
   sockaddr_un structure should have a value of at least:

   offsetof(struct sockaddr_un, sun_path) +
   strlen(addr.sun_path)+1

   or,  more  simply,  addrlen  can   be   specified   as
   sizeof(struct sockaddr_un).

So let's follow the last advice and simplify the code as well.

Signed-off-by: Stefano Garzarella 
---
  contrib/vhost-user-blk/vhost-user-blk.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé

[PATCH for-9.1 v3 05/11] contrib/vhost-user-blk: fix bind() using the right size of the address

2024-04-04 Thread Stefano Garzarella 
On macOS passing `-s /tmp/vhost.socket` parameter to the vhost-user-blk
application, the bind was done on `/tmp/vhost.socke` pathname,
missing the last character.

This sounds like one of the portability problems described in the
unix(7) manpage:

   Pathname sockets
   When  binding  a socket to a pathname, a few rules should
   be observed for maximum portability and ease of coding:

   •  The pathname in sun_path should be null-terminated.

   •  The length of the pathname, including the  terminating
  null byte, should not exceed the size of sun_path.

   •  The  addrlen  argument  that  describes  the enclosing
  sockaddr_un structure should have a value of at least:

  offsetof(struct sockaddr_un, sun_path) +
  strlen(addr.sun_path)+1

  or,  more  simply,  addrlen  can   be   specified   as
  sizeof(struct sockaddr_un).

So let's follow the last advice and simplify the code as well.

Signed-off-by: Stefano Garzarella 
---
 contrib/vhost-user-blk/vhost-user-blk.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/contrib/vhost-user-blk/vhost-user-blk.c 
b/contrib/vhost-user-blk/vhost-user-blk.c
index 89e5f11a64..a8ab9269a2 100644
--- a/contrib/vhost-user-blk/vhost-user-blk.c
+++ b/contrib/vhost-user-blk/vhost-user-blk.c
@@ -469,7 +469,6 @@ static int unix_sock_new(char *unix_fn)
 {
 int sock;
 struct sockaddr_un un;
-size_t len;
 
 assert(unix_fn);
 
@@ -481,10 +480,9 @@ static int unix_sock_new(char *unix_fn)
 
 un.sun_family = AF_UNIX;
 (void)snprintf(un.sun_path, sizeof(un.sun_path), "%s", unix_fn);
-len = sizeof(un.sun_family) + strlen(un.sun_path);
 
 (void)unlink(unix_fn);
-if (bind(sock, (struct sockaddr *), len) < 0) {
+if (bind(sock, (struct sockaddr *), sizeof(un)) < 0) {
 perror("bind");
 goto fail;
 }
-- 
2.44.0