[systemd-devel] [PATCH] [RFC] [WIP] [kdbus] Attempt to recursively pass fd

[Alban Crequy]

Before Linux commit 25888e (from 2.6.37-rc4, Nov 2010), fd-passing on Unix
sockets could recursively be stacked, allowing a process to exhaust the open
files limit (/proc/sys/fs/file-max) on the system without restriction from
ulimit -n.

This DoS on Unix sockets was fixed by commit:

> commit 25888e30319f8896fc656fc68643e6a078263060
> Author: Eric Dumazet <eric.dumazet at gmail.com>
> Date:   Thu Nov 25 04:11:39 2010 +0000
>
>     af_unix: limit recursion level

But that commit introduced a bug in dbus:
https://bugs.freedesktop.org/show_bug.cgi?id=80163

kdbus does not use fd-passing on Unix sockets so it is not affected by this.

However, it allows fd-passing similarly. This patch shows it is possible to
recursively pass file descriptors in kdbus and stack them without keeping them
attached to the initial process. I could stack passed fds 256 times, probably
because of the limit KDBUS_USER_MAX_CONN:

defaults.h:#define KDBUS_USER_MAX_CONN                  256

But this limit could probably be overrided by using fds from different
endpoints.

I am also afraid that fds from Unix sockets and fds from kdbus could be stacked
together, defeating the limit implemented in unix_attach_fds() by commit 25888e
because the check uses unix_get_socket(), making the assumption that only Unix
sockets could carry file descriptors:
http://lxr.free-electrons.com/source/net/unix/af_unix.c#L1380
http://lxr.free-electrons.com/source/net/unix/garbage.c#L99
---
 test/test-kdbus.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/test/test-kdbus.c b/test/test-kdbus.c
index f0bf705..3c79877 100644
--- a/test/test-kdbus.c
+++ b/test/test-kdbus.c
@@ -12,6 +12,8 @@
 #include <limits.h>
 #include <sys/mman.h>
 #include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/socket.h>
 #include <getopt.h>
 #include <stdbool.h>
 
@@ -1004,6 +1006,75 @@ static int check_msg_basic(struct kdbus_check_env *env)
 	return CHECK_OK;
 }
 
+static int send_fds(struct conn *conn, uint64_t dst_id, int fds[2])
+{
+	struct kdbus_msg *msg;
+	struct kdbus_item *item;
+	uint64_t size;
+	int ret;
+
+	size = sizeof(struct kdbus_msg);
+	size += KDBUS_ITEM_SIZE(sizeof(int[2]));
+
+	msg = malloc(size);
+	ASSERT_RETURN (msg != NULL);
+
+	memset(msg, 0, size);
+	msg->size = size;
+	msg->src_id = conn->id;
+	msg->dst_id = dst_id;
+	msg->payload_type = KDBUS_PAYLOAD_DBUS;
+
+	item = msg->items;
+
+	item->type = KDBUS_ITEM_FDS;
+	item->size = KDBUS_ITEM_HEADER_SIZE + sizeof(int[2]);
+	item->fds[0] = fds[0];
+	item->fds[1] = fds[1];
+	item = KDBUS_ITEM_NEXT(item);
+
+	ret = ioctl(conn->fd, KDBUS_CMD_MSG_SEND, msg);
+	if (ret) {
+		fprintf(stderr, "error sending message: %d err %d (%m)\n", ret, errno);
+		return EXIT_FAILURE;
+	}
+
+	free(msg);
+
+	return 0;
+}
+
+static int check_fds_passing(struct kdbus_check_env *env)
+{
+	struct conn *conn_src, *conn_dst;
+	int fds[2];
+	int ret;
+	int i;
+
+	/* create two connections */
+	conn_src = kdbus_hello(env->buspath, 0, NULL, 0);
+	ASSERT_RETURN(conn_src != NULL);
+
+	for (i = 0; i >= 0; i++) {
+		conn_dst = kdbus_hello(env->buspath, 0, NULL, 0);
+		ASSERT_RETURN(conn_dst != NULL);
+
+		fds[0] = conn_src->fd;
+		fds[1] = conn_dst->fd;
+
+		ret = send_fds (conn_src, conn_dst->id, fds);
+		printf("check_fds_passing: iter: %d fds %d-%d ret %d err %d (%m)\n", i, fds[0], fds[1], ret, errno);
+		ASSERT_RETURN(ret == 0);
+
+		close(conn_src->fd);
+		free(conn_src);
+
+		conn_src = conn_dst;
+	}
+
+	return CHECK_OK;
+}
+
 static int check_msg_free(struct kdbus_check_env *env)
 {
 	int ret;
@@ -1111,6 +1182,7 @@ static const struct kdbus_check checks[] = {
 	{ "name queue",		check_name_queue,		CHECK_CREATE_BUS | CHECK_CREATE_CONN	},
 	{ "message basic",	check_msg_basic,		CHECK_CREATE_BUS | CHECK_CREATE_CONN	},
 	{ "message free",	check_msg_free,			CHECK_CREATE_BUS | CHECK_CREATE_CONN	},
+	{ "fds passing",	check_fds_passing,		CHECK_CREATE_BUS                        },
 	{ "connection info",	check_conn_info,		CHECK_CREATE_BUS | CHECK_CREATE_CONN	},
 	{ "match id add",	check_match_id_add,		CHECK_CREATE_BUS | CHECK_CREATE_CONN	},
 	{ "match id remove",	check_match_id_remove,		CHECK_CREATE_BUS | CHECK_CREATE_CONN	},
-- 
1.8.5.3