fd passing

[mischief]

hi,

i saw there was some support for encoding/decoding dbus fd passing, but my attempt to make it work failed.

i tried to implement a test within the test framework using luaposix, and the fd seems to be passed from the server side correctly, but on the client side the fd is closed before the handler receives it. i'm suspecting that there needs to be better handling around managing file descriptor lifetimes.

in my test, i am only encoding an fd in a reply from server side and receiving on client side. in the docs, for sd_bus_message_append_basic, it says that SD_BUS_TYPE_UNIX_FD, it will duplicate the fd and the caller needs to close. however, in the server side handler, the fd can only be added to the message through return, so the fd can leak unless we asynchronously close it after return. maybe via adding sd_event_add_defer support, but that seems clunky - lsdbus api should probably handle that.

on the client side, using sd_bus_message_read_basic, the doc says for SD_BUS_TYPE_UNIX_FD that the caller is responsible for duplicating the fd and the fd will be closed when the message is unreferenced. i observe in my test that the fd seems to be closed by libsystemd before the lsdbus proxy client gets a chance to duplicate it unfortunately.

below is my unsuccessful test and a fragment of strace showing the successful fd pass from the server, but closing before dup in the client:

diff --git a/test/peer-testserver.lua b/test/peer-testserver.lua
index 5d4fa39..cbcb394 100644
--- a/test/peer-testserver.lua
+++ b/test/peer-testserver.lua
@@ -115,6 +115,21 @@ local interface = {
               end
               return creds.euid, creds.pid
           end,
+      },
+
+      GetFD = {
+          { direction = "out", name = "fd", type = "h" },
+          handler = function(vt)
+	      local unistd_ok, unistd = pcall(require, "posix.unistd")
+	      if not unistd_ok then
+                  return -1
+              end
+
+              local p1, p2 = unistd.pipe()
+	      unistd.write(p2, "fnord")
+	      unistd.close(p2)
+	      return p1
+          end
       }
    },
    properties={
diff --git a/test/testserver.lua b/test/testserver.lua
index 6f63f14..32e1f87 100644
--- a/test/testserver.lua
+++ b/test/testserver.lua
@@ -428,4 +428,18 @@ function TestServer:TestCallAV()
 
 end
 
+function TestServer:TestFDPass()
+   local unistd_ok, unistd = pcall(require, "posix.unistd")
+   if not unistd_ok then
+      return lu.skip("no luaposix")
+   end
+
+   local fd = p1('GetFD')
+   local dup, err = unistd.dup(fd)
+   lu.assert_nil(err)
+   local b, err = unistd.read(dup, 128)
+   lu.assert_nil(err)
+   lu.assert_equals(b, "fnord")
+end
+
 return TestServer

[pid 2075524] pipe2([8, 9], 0)          = 0
[pid 2075524] write(9, "fnord", 5)      = 5
[pid 2075524] close(9)                  = 0
[pid 2075524] fcntl(8, F_DUPFD_CLOEXEC, 3) = 9
[pid 2075524] sendmsg(3, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="l\2\1\1\4\0\0\0\t\0\0\0(\0\0\0\5\1u\0\5\0\0\0\6\1s\0\6\0\0\0"..., iov_len=56}, {iov_base="\0\0\0\0", iov_len=4}], msg_iovlen=2, msg_contr
ol=[{cmsg_len=20, cmsg_level=SOL_SOCKET, cmsg_type=SCM_RIGHTS, cmsg_data=[9]}], msg_controllen=24, msg_flags=0}, MSG_DONTWAIT|MSG_NOSIGNAL) = 60
[pid 2075524] close(9 <unfinished ...>
[pid 2075526] <... ppoll resumed>)      = 1 ([{fd=3, revents=POLLIN}], left {tv_sec=24, tv_nsec=999060706})
[pid 2075524] <... close resumed>)      = 0
[pid 2075526] recvmsg(3 <unfinished ...>
[pid 2075524] epoll_wait(5 <unfinished ...>
[pid 2075526] <... recvmsg resumed>, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="l\2\1\1\4\0\0\0\t\0\0\0007\0\0\0\5\1u\0\5\0\0\0", iov_len=24}], msg_iovlen=1, msg_control=[{cmsg_len=20, cmsg_level=SOL_SOCKE
T, cmsg_type=SCM_RIGHTS, cmsg_data=[5]}], msg_controllen=24, msg_flags=MSG_CMSG_CLOEXEC}, MSG_DONTWAIT|MSG_CMSG_CLOEXEC) = 24
[pid 2075526] recvmsg(3, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="\6\1s\0\6\0\0\0:1.202\0\0\10\1g\0\1h\0\0\t\1u\0\1\0\0\0"..., iov_len=52}], msg_iovlen=1, msg_controllen=0, msg_flags=MSG_CMSG_CLOEXEC}, M
SG_DONTWAIT|MSG_CMSG_CLOEXEC) = 52
[pid 2075526] close(5)                  = 0
[pid 2075526] dup(5)                    = -1 EBADF (Bad file descriptor)

to fix this i think the API needs some improved semantics around fd handling/SD_BUS_TYPE_UNIX_FD. what do you think?

[mischief]

it seems we need to dup the fd before deallocating the sd bus message. this seems to work:

diff --git a/src/message.c b/src/message.c
index d35c8e6..6e9ec9a 100644
--- a/src/message.c
+++ b/src/message.c
@@ -739,9 +739,17 @@ static int __msg_tolua(lua_State *L, sd_bus_message* m, char ctype, int raw)
 			lua_pushinteger(L, basic.u16);
                         break;
 
-                case SD_BUS_TYPE_INT32:
                 case SD_BUS_TYPE_UNIX_FD:
-			dbg("push INT32/FD %i", basic.s32);
+			dbg("push FD %i", basic.i);
+			int r = fcntl(basic.i, F_DUPFD_CLOEXEC, 3);
+			if (r < 0)
+				luaL_error(L, "msg_tolua: fcntl error: %s", strerror(-r));
+
+			lua_pushinteger(L, r);
+			break;
+
+                case SD_BUS_TYPE_INT32:
+			dbg("push INT32 %i", basic.s32);
 			lua_pushinteger(L, basic.s32);
 			break;