fd passing functionality fails on x86_64

[korli]

Dovecot requires the ability to pass a file descriptor between two processes.

This is done through local network sockets and looks implemented in Haiku and even works on x86 (see ​here )

gcc -o conftest -std=gnu99 -D_BSD_SOURCE -Wall -W -Wmissing-prototypes -Wmissing-declarations -Wpointer-arith -Wchar-subscripts -Wformat=2 -Wbad-function-cast -fno-builtin-strftime -Wstrict-aliasing=2 fdpass.c  conftest.c -lnetwork

conftest exits with code 2 on x86_64. Attached are conftest.c and fdpass.c as example of checking the functionality.

[korli]

Following patch fixes the issue:

diff --git a/src/add-ons/kernel/network/stack/net_socket.cpp b/src/add-ons/kernel/network/stack/net_socket.cpp
index 5582351..b447a99 100644
--- a/src/add-ons/kernel/network/stack/net_socket.cpp
+++ b/src/add-ons/kernel/network/stack/net_socket.cpp
@@ -222,7 +222,7 @@ add_ancillary_data(net_socket* socket, ancillary_data_container* container,
        cmsghdr* header = (cmsghdr*)data;
 
        while (dataLen > 0) {
-               if (header->cmsg_len < sizeof(cmsghdr) || header->cmsg_len > dataLen)
+               if (header->cmsg_len < _ALIGN(sizeof(cmsghdr)) || header->cmsg_len > dataLen)
                        return B_BAD_VALUE;
 
                if (socket->first_info->add_ancillary_data == NULL)
@@ -233,8 +233,8 @@ add_ancillary_data(net_socket* socket, ancillary_data_container* container,
                if (status != B_OK)
                        return status;
 
-               dataLen -= _ALIGN(header->cmsg_len);
-               header = (cmsghdr*)((uint8*)header + _ALIGN(header->cmsg_len));
+               dataLen -= header->cmsg_len;
+               header = (cmsghdr*)((uint8*)header + header->cmsg_len);
        }
 

Though it might be more correct to change dataLen to a ssize_t.

Opinions?

[axeld]

Nice catch. I think the code would look better if you replaced the if with:

if (header->cmsg_len < CMSG_LEN(0) || header->cmsg_len > datalen)

[axeld]

Oh, about the ssize_t: I think it's fine the way it is now. The if-clause makes sure that dataLen cannot drop below zero, and you don't want to accept negative values there, anyway.

[korli]

Thinking of it, I removed _ALIGN(), but I don't even know whether it's relevant, it might make sense to keep aligning the buffer. dataLen is actually overflown, we just need to check whether dataLen is big enough before doing the substract. Also it might make sense to check if dataLen is at least big enough to hold a header (sizeof(cmsghdr)).

 static status_t
 add_ancillary_data(net_socket* socket, ancillary_data_container* container,
        void* data, size_t dataLen)
 {
        cmsghdr* header = (cmsghdr*)data;
 
-       while (dataLen > 0) {
-               if (header->cmsg_len < sizeof(cmsghdr) || header->cmsg_len > dataLen)
+       if (dataLen == 0)
+               return B_OK;
+
+       while (true) {
+               if (header->cmsg_len < CMSG_LEN(0) || header->cmsg_len > dataLen)
                        return B_BAD_VALUE;
 
                if (socket->first_info->add_ancillary_data == NULL)
                        return B_NOT_SUPPORTED;
 
                status_t status = socket->first_info->add_ancillary_data(
                        socket->first_protocol, container, header);
                if (status != B_OK)
                        return status;
 
+               if (dataLen <= _ALIGN(header->cmsg_len))
+                       break;
                dataLen -= _ALIGN(header->cmsg_len);
                header = (cmsghdr*)((uint8*)header + _ALIGN(header->cmsg_len));
        }
 
        return B_OK;
  }

[axeld]

Ah, I misunderstood before. You're right, the CMSG_NXTHDR() macro definition indicates that the _ALIGN() in this case is actually needed.

BTW, when you touch that method anyway, I would move the test for the availability of the add_anillary_data function in the protocol out of the loop.

[korli]

Fixed in hrev50266.