[PATCH] dbus: Use KDBUS_CMD_FREE after kdbus hello

[PATCH] dbus: Use KDBUS_CMD_FREE after kdbus hello

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 1730 bytes --]

---
 ell/dbus-kernel.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/ell/dbus-kernel.c b/ell/dbus-kernel.c
index 9bc99d7..1c0fbb6 100644
--- a/ell/dbus-kernel.c
+++ b/ell/dbus-kernel.c
@@ -237,6 +237,7 @@ int _dbus_kernel_hello(int fd, const char *connection_name,
 	struct kdbus_cmd_hello *hello;
 	struct kdbus_item *item;
 	int ret;
+	struct kdbus_cmd_free cmd_free;
 
 	size = align_len(sizeof(struct kdbus_cmd_hello), 8);
 	size += KDBUS_ITEM_SIZE(len + 1);
@@ -256,18 +257,23 @@ int _dbus_kernel_hello(int fd, const char *connection_name,
 	strcpy(item->str, connection_name);
 
 	ret = ioctl(fd, KDBUS_CMD_HELLO, hello);
-	if (ret < 0)
-		return -errno;
+	if (ret < 0) {
+		ret = -errno;
+		goto done;
+	}
 
         /* Check for incompatible flags (upper 32 bits) */
         if (hello->bus_flags > 0xFFFFFFFFULL ||
-			hello->flags > 0xFFFFFFFFULL)
-                return -ENOTSUP;
+			hello->flags > 0xFFFFFFFFULL) {
+		ret = -ENOTSUP;
+		goto done;
+	}
 
 	*pool = mmap(NULL, KDBUS_POOL_SIZE, PROT_READ, MAP_SHARED, fd, 0);
 	if (*pool == MAP_FAILED) {
 		*pool = NULL;
-		return -errno;
+		ret = -errno;
+		goto done;
 	}
 
 	*bloom_size = DEFAULT_BLOOM_SIZE;
@@ -286,7 +292,14 @@ int _dbus_kernel_hello(int fd, const char *connection_name,
 				hello->id128[12], hello->id128[13],
 				hello->id128[14], hello->id128[15]);
 
-	return 0;
+done:
+	memset(&cmd_free, 0, sizeof(cmd_free));
+	cmd_free.size = sizeof(cmd_free);
+	cmd_free.offset = hello->offset;
+
+	ioctl(fd, KDBUS_CMD_FREE, &cmd_free);
+
+	return ret;
 }
 
 int _dbus_kernel_send(int fd, size_t bloom_size, uint8_t bloom_n_hash,
-- 
2.5.0

[PATCH] dbus: Return the bloom parameters in _dbus_kernel_hello

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 759 bytes --]

---
 ell/dbus-kernel.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/ell/dbus-kernel.c b/ell/dbus-kernel.c
index 1c0fbb6..b31cfcc 100644
--- a/ell/dbus-kernel.c
+++ b/ell/dbus-kernel.c
@@ -279,6 +279,15 @@ int _dbus_kernel_hello(int fd, const char *connection_name,
 	*bloom_size = DEFAULT_BLOOM_SIZE;
 	*bloom_n_hash = DEFAULT_BLOOM_N_HASH;
 
+	KDBUS_ITEM_FOREACH(item, hello, items) {
+		switch (item->type) {
+		case KDBUS_ITEM_BLOOM_PARAMETER:
+			*bloom_size = item->bloom_parameter.size;
+			*bloom_n_hash = item->bloom_parameter.n_hash;
+			break;
+		}
+	}
+
 	*id = hello->id;
 	*guid = l_strdup_printf("%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
 				"%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
-- 
2.5.0

Re: [PATCH] dbus: Return the bloom parameters in _dbus_kernel_hello

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 190 bytes --]

Hi Andrew,

On 03/31/2016 09:41 PM, Andrew Zaborowski wrote:
> ---
>   ell/dbus-kernel.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
>

Applied, thanks.

Regards,
-Denis

[PATCH] dbus: Set a 30s kdbus method call timeout

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 579 bytes --]

---
 ell/dbus-kernel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ell/dbus-kernel.c b/ell/dbus-kernel.c
index b31cfcc..f828657 100644
--- a/ell/dbus-kernel.c
+++ b/ell/dbus-kernel.c
@@ -393,7 +393,7 @@ int _dbus_kernel_send(int fd, size_t bloom_size, uint8_t bloom_n_hash,
 	}
 	case DBUS_MESSAGE_TYPE_METHOD_CALL:
 		if (!l_dbus_message_get_no_reply(message))
-			kmsg->timeout_ns = 30000 * 1000ULL;
+			kmsg->timeout_ns = 30000 * 1000000ULL;
 		break;
 	case DBUS_MESSAGE_TYPE_SIGNAL:
 		kmsg->flags |= KDBUS_MSG_SIGNAL;
-- 
2.5.0

Re: [PATCH] dbus: Set a 30s kdbus method call timeout

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 197 bytes --]

Hi Andrew,

On 03/31/2016 09:41 PM, Andrew Zaborowski wrote:
> ---
>   ell/dbus-kernel.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>

Applied, thanks.

Regards,
-Denis

[PATCH] dbus: Fix _dbus_kernel_calculate_bloom for multiple arguments

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 2353 bytes --]

l_dbus_message_iter_get_type can't be used to get the value of a
single argument and be called repeatedly, might crash if the number
and types of call arguments don't match the message arguments.
Add a comment explaining why arguments after first non-string argument
are not considered.
---
 ell/dbus-message.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/ell/dbus-message.c b/ell/dbus-message.c
index 4d64115..ece98a4 100644
--- a/ell/dbus-message.c
+++ b/ell/dbus-message.c
@@ -1427,6 +1427,7 @@ bool _dbus_kernel_calculate_bloom(struct l_dbus_message *message,
 	struct l_dbus_message_iter iter;
 	uint8_t argn;
 	char buf[256];
+	bool (*get_basic)(struct l_dbus_message_iter *, char, void *);
 
 	/* The string "interface:" suffixed by the interface name */
 	attr = l_dbus_message_get_interface(message);
@@ -1470,17 +1471,36 @@ bool _dbus_kernel_calculate_bloom(struct l_dbus_message *message,
 
 	body = _dbus_message_get_body(message, &body_size);
 
-	if (_dbus_message_is_gvariant(message))
-		_gvariant_iter_init(&iter, message, signature, NULL,
-					body, body_size);
-	else
+	if (_dbus_message_is_gvariant(message)) {
+		if (!_gvariant_iter_init(&iter, message, signature, NULL,
+						body, body_size))
+			return false;
+
+		get_basic = _gvariant_iter_next_entry_basic;
+	} else {
 		_dbus1_iter_init(&iter, message, signature, NULL,
-				body, body_size);
+					body, body_size);
+
+		get_basic = _dbus1_iter_next_entry_basic;
+	}
 
 	argn = 0;
 
+	/*
+	 * systemd-master/src/libsystemd/sd-bus/PORTING-DBUS1:
+	 *
+	 * "If the first argument of the message is a string,
+	 * "arg0-slash-prefix" suffixed with the first argument, and also
+	 * all prefixes of the argument (cut off at "/"), also prefixed
+	 * with "arg0-slash-prefix".
+	 *
+	 * Similar for all further arguments that are strings up to 63,
+	 * for the arguments and their "dot" and "slash" prefixes. On the
+	 * first argument that is not a string, addition to the bloom
+	 * filter should be stopped however."
+	 */
 	while (*signature == 's' || *signature == 'o' || *signature == 'g') {
-		if (!message_iter_next_entry(&iter, &attr))
+		if (!get_basic(&iter, *signature, &attr))
 			return false;
 
 		sprintf(buf, "arg%hhu", argn);
-- 
2.5.0

Re: [PATCH] dbus: Fix _dbus_kernel_calculate_bloom for multiple arguments

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 531 bytes --]

Hi Andrew,

On 03/31/2016 09:41 PM, Andrew Zaborowski wrote:
> l_dbus_message_iter_get_type can't be used to get the value of a
> single argument and be called repeatedly, might crash if the number
> and types of call arguments don't match the message arguments.
> Add a comment explaining why arguments after first non-string argument
> are not considered.
> ---
>   ell/dbus-message.c | 32 ++++++++++++++++++++++++++------
>   1 file changed, 26 insertions(+), 6 deletions(-)

Applied, thanks.

Regards,
-Denis

Re: [PATCH] dbus: Use KDBUS_CMD_FREE after kdbus hello

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 224 bytes --]

Hi Andrew,

On 03/31/2016 09:41 PM, Andrew Zaborowski wrote:
> ---
>   ell/dbus-kernel.c | 25 +++++++++++++++++++------
>   1 file changed, 19 insertions(+), 6 deletions(-)
>

Applied, thanks.

Regards,
-Denis