1. Patchew
  2. QEMU
  3. [PATCH v2 0/2] net/tap: minor fixes to interaction with the bridge helper
  4. View patch

[PATCH v2 1/2] net/tap: fix FreeBSD portability problem receiving TAP FD

Daniel P. Berrangé posted 2 patches 4 years, 9 months ago
[PATCH v2 1/2] net/tap: fix FreeBSD portability problem receiving TAP FD
Posted by Daniel P. Berrangé 4 years, 9 months ago 
The CMSG_LEN and CMSG_SPACE macros must not be assumed to return the
same value. The msg_controllen field must be initialized using
CMSG_SPACE when using SCM_RIGHTS.

This ought to fix any FD receive issues users might be hitting on
64-bit FeeBSD / NetBSD platforms. The flaw was noticed first in
GNULIB

   https://lists.gnu.org/archive/html/bug-gnulib/2021-02/msg00066.html

and QEMU's code has the same logic bug.

Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 net/tap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/tap.c b/net/tap.c
index bae895e287..276a9077fc 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -467,7 +467,7 @@ static int recv_fd(int c)
     cmsg->cmsg_level = SOL_SOCKET;
     cmsg->cmsg_type = SCM_RIGHTS;
     cmsg->cmsg_len = CMSG_LEN(sizeof(fd));
-    msg.msg_controllen = cmsg->cmsg_len;
+    msg.msg_controllen = CMSG_SPACE(sizeof(fd));
 
     iov.iov_base = req;
     iov.iov_len = sizeof(req);
-- 
2.31.1
Re: [PATCH v2 1/2] net/tap: fix FreeBSD portability problem receiving TAP FD
Posted by Philippe Mathieu-Daudé 4 years, 9 months ago 
On 5/12/21 5:36 PM, Daniel P. Berrangé wrote:
> The CMSG_LEN and CMSG_SPACE macros must not be assumed to return the
> same value. The msg_controllen field must be initialized using
> CMSG_SPACE when using SCM_RIGHTS.
> 
> This ought to fix any FD receive issues users might be hitting on
> 64-bit FeeBSD / NetBSD platforms. The flaw was noticed first in
> GNULIB
> 
>    https://lists.gnu.org/archive/html/bug-gnulib/2021-02/msg00066.html
> 
> and QEMU's code has the same logic bug.
> 
> Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  net/tap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/tap.c b/net/tap.c
> index bae895e287..276a9077fc 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -467,7 +467,7 @@ static int recv_fd(int c)
>      cmsg->cmsg_level = SOL_SOCKET;
>      cmsg->cmsg_type = SCM_RIGHTS;
>      cmsg->cmsg_len = CMSG_LEN(sizeof(fd));
> -    msg.msg_controllen = cmsg->cmsg_len;
> +    msg.msg_controllen = CMSG_SPACE(sizeof(fd));
>  
>      iov.iov_base = req;
>      iov.iov_len = sizeof(req);
> 

Possibly more:

-- >8 --
diff --git a/contrib/ivshmem-client/ivshmem-client.c
b/contrib/ivshmem-client/ivshmem-client.c
index 182c79d27cf..f4ee226deba 100644
--- a/contrib/ivshmem-client/ivshmem-client.c
+++ b/contrib/ivshmem-client/ivshmem-client.c
@@ -44 +44 @@ ivshmem_client_read_one_msg(IvshmemClient *client, int64_t
*index, int *fd)
-    msg.msg_controllen = sizeof(msg_control);
+    msg.msg_controllen = CMSG_SPACE(sizeof(msg_control));
diff --git a/contrib/ivshmem-server/ivshmem-server.c
b/contrib/ivshmem-server/ivshmem-server.c
index 39a6ffdb5df..209eb120716 100644
--- a/contrib/ivshmem-server/ivshmem-server.c
+++ b/contrib/ivshmem-server/ivshmem-server.c
@@ -55 +55 @@ ivshmem_server_send_one_msg(int sock_fd, int64_t peer_id,
int fd)
-        msg.msg_controllen = sizeof(msg_control);
+        msg.msg_controllen = CMSG_SPACE(sizeof(msg_control));
diff --git a/contrib/vhost-user-gpu/vhost-user-gpu.c
b/contrib/vhost-user-gpu/vhost-user-gpu.c
index f73f292c9f7..e72a59b135a 100644
--- a/contrib/vhost-user-gpu/vhost-user-gpu.c
+++ b/contrib/vhost-user-gpu/vhost-user-gpu.c
@@ -161 +161 @@ vg_sock_fd_write(int sock, const void *buf, ssize_t
buflen, int fd)
-        msg.msg_controllen = sizeof(cmsgu.control);
+        msg.msg_controllen = CMSG_SPACE(sizeof(cmsgu.control));
diff --git a/io/channel-socket.c b/io/channel-socket.c
index de259f7eed2..753d0d2f8ef 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -498 +498 @@ static ssize_t qio_channel_socket_readv(QIOChannel *ioc,
-        msg.msg_controllen = sizeof(control);
+        msg.msg_controllen = CMSG_SPACE(sizeof(control));
diff --git a/net/tap.c b/net/tap.c
index bae895e2874..05ab4c8a530 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -459 +459 @@ static int recv_fd(int c)
-        .msg_controllen = sizeof(msgbuf),
+        .msg_controllen = CMSG_SPACE(sizeof(msgbuf)),
diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
index a26e1663f02..983b4347523 100644
--- a/qemu-bridge-helper.c
+++ b/qemu-bridge-helper.c
@@ -189 +189 @@ static int send_fd(int c, int fd)
-        .msg_controllen = sizeof(msgbuf),
+        .msg_controllen = CMSG_SPACE(sizeof(msgbuf)),
@@ -199 +199 @@ static int send_fd(int c, int fd)
-    msg.msg_controllen = cmsg->cmsg_len;
+    msg.msg_controllen = CMSG_SPACE(sizeof(fd));
diff --git a/subprojects/libvhost-user/libvhost-user.c
b/subprojects/libvhost-user/libvhost-user.c
index fab7ca17ee1..f083db4ea16 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -281 +281 @@ vu_message_read_default(VuDev *dev, int conn_fd,
VhostUserMsg *vmsg)
-        .msg_controllen = sizeof(control),
+        .msg_controllen = CMSG_SPACE(sizeof(control)),
diff --git a/tests/qemu-iotests/socket_scm_helper.c
b/tests/qemu-iotests/socket_scm_helper.c
index eb76d31aa94..aeba41edea0 100644
--- a/tests/qemu-iotests/socket_scm_helper.c
+++ b/tests/qemu-iotests/socket_scm_helper.c
@@ -42 +42 @@ static int send_fd(int fd, int fd_to_send)
-    msg.msg_controllen = sizeof(control);
+    msg.msg_controllen = CMSG_SPACE(sizeof(control));
---
Re: [PATCH v2 1/2] net/tap: fix FreeBSD portability problem receiving TAP FD
Posted by Daniel P. Berrangé 4 years, 9 months ago 
On Wed, May 12, 2021 at 06:35:57PM +0200, Philippe Mathieu-Daudé wrote:
> On 5/12/21 5:36 PM, Daniel P. Berrangé wrote:
> > The CMSG_LEN and CMSG_SPACE macros must not be assumed to return the
> > same value. The msg_controllen field must be initialized using
> > CMSG_SPACE when using SCM_RIGHTS.
> > 
> > This ought to fix any FD receive issues users might be hitting on
> > 64-bit FeeBSD / NetBSD platforms. The flaw was noticed first in
> > GNULIB
> > 
> >    https://lists.gnu.org/archive/html/bug-gnulib/2021-02/msg00066.html
> > 
> > and QEMU's code has the same logic bug.
> > 
> > Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  net/tap.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/net/tap.c b/net/tap.c
> > index bae895e287..276a9077fc 100644
> > --- a/net/tap.c
> > +++ b/net/tap.c
> > @@ -467,7 +467,7 @@ static int recv_fd(int c)
> >      cmsg->cmsg_level = SOL_SOCKET;
> >      cmsg->cmsg_type = SCM_RIGHTS;
> >      cmsg->cmsg_len = CMSG_LEN(sizeof(fd));
> > -    msg.msg_controllen = cmsg->cmsg_len;
> > +    msg.msg_controllen = CMSG_SPACE(sizeof(fd));
> >  
> >      iov.iov_base = req;
> >      iov.iov_len = sizeof(req);
> > 
> 
> Possibly more:

Ewwwww, yes :-(



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.