[systemd-devel] [PATCH 1/2] connection: fix cpu stall by checking kdbus item alignment

[Djalal Harouni]

Fix the following stall triggered by a policy holder hello:
call test/kdbus-util.c:kdbus_hello_registrar() and pass
KDBUS_HELLO_POLICY_HOLDER as a flag.

While we are it make sure that kdbus_cmd_conn_update() also checks for
proper 8 byte alignment.

[  142.731011] INFO: rcu_sched self-detected stall on CPU { 3}  (t=65000 jiffies g=3085 c=3084 q=4316)
[  142.731011] sending NMI to all CPUs:
[  142.731011] NMI backtrace for cpu 3
[  142.731011] CPU: 3 PID: 1352 Comm: test-kdbus-poli Tainted: G OE 3.15.0-0.rc8.git4.1.fc21.x86_64 #1
[  142.731011] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[  142.731011] task: ffff88005a2719d0 ti: ffff88004587c000 task.ti: ffff88004587c000
[  142.731011] RIP: 0010:[<ffffffff81055a12>]  [<ffffffff81055a12>] flat_send_IPI_all+0x92/0xd0
[  142.731011] RSP: 0018:ffff88005dc03dc0  EFLAGS: 00010006
[  142.731011] RAX: 0000000000000000 RBX: 0000000000000c00 RCX: 0000000000000000
[  142.731011] RDX: 0000000000000c00 RSI: 0000000000000000 RDI: 0000000000000300
[  142.731011] RBP: ffff88005dc03dd8 R08: 0000000000000001 R09: 0000000000000001
[  142.731011] R10: ffffffff81e5d720 R11: 0000000000000001 R12: 0000000000000046
[  142.731011] R13: 000000000000000f R14: ffffffff81e86880 R15: 0000000000000003
[  142.731011] FS:  00007f4b9d727740(0000) GS:ffff88005dc00000(0000) knlGS:0000000000000000
[  142.731011] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[  142.731011] CR2: 0000000000401312 CR3: 000000005b66f000 CR4: 00000000000006e0
[  142.731011] Stack:
[  142.731011]  0000000000002710 ffffffff81e86880 ffffffff81fa3e40 ffff88005dc03df0
[  142.731011]  ffffffff81050654 ffff88005ddcf000 ffff88005dc03e48 ffffffff8111a194
[  142.731011]  0000000000000000 7fffffffffffffff 0000000000000046 00000000000010dc
[  142.731011] Call Trace:
[  142.731011]  <IRQ>

[  142.731011]  [<ffffffff81050654>] arch_trigger_all_cpu_backtrace+0x64/0xa0
[  142.731011]  [<ffffffff8111a194>] rcu_check_callbacks+0x584/0x850
[  142.731011]  [<ffffffff810a74a7>] update_process_times+0x47/0x70
[  142.731011]  [<ffffffff81126765>] tick_sched_handle.isra.19+0x25/0x60
[  142.731011]  [<ffffffff81127061>] tick_sched_timer+0x41/0x60
[  142.731011]  [<ffffffff810c7446>] __run_hrtimer+0x86/0x460
[  142.731011]  [<ffffffff81127020>] ? tick_sched_do_timer+0x40/0x40
[  142.731011]  [<ffffffff810c809f>] hrtimer_interrupt+0x10f/0x260
[  142.731011]  [<ffffffff8104e55a>] local_apic_timer_interrupt+0x3a/0x60
[  142.731011]  [<ffffffff8180089f>] smp_apic_timer_interrupt+0x3f/0x50
[  142.731011]  [<ffffffff817ff1f2>] apic_timer_interrupt+0x72/0x80
[  142.731011]  <EOI>

[  142.731011]  [<ffffffff817f42b3>] ? retint_restore_args+0x13/0x13
[  142.731011]  [<ffffffffa02a666b>] ? kdbus_conn_new+0x25b/0xf20 [kdbus]
[  142.731011]  [<ffffffffa02a6741>] ? kdbus_conn_new+0x331/0xf20 [kdbus]
[  142.731011]  [<ffffffffa02a8161>] kdbus_handle_ioctl+0x221/0xad0 [kdbus]
[  142.731011]  [<ffffffff81361a31>] ? inode_has_perm.isra.47+0x51/0x90
[  142.731011]  [<ffffffff81251f60>] do_vfs_ioctl+0x2f0/0x520
[  142.731011]  [<ffffffff81252211>] SyS_ioctl+0x81/0xa0
[  142.731011]  [<ffffffff817fe4e9>] system_call_fastpath+0x16/0x1b

Signed-off-by: Djalal Harouni <tixxdz at opendz.org>
---
I've checked all the other calls, the remaining one is:
connection.c:kdbus_conn_payload_add() it seems that it fakes the size
and hanldes the alignment, if I've more time I'll try to check it. This
one is tricky!

 connection.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/connection.c b/connection.c
index 3f27889..3e8c5de 100644
--- a/connection.c
+++ b/connection.c
@@ -1784,12 +1784,18 @@ int kdbus_cmd_conn_update(struct kdbus_conn *conn,
 {
 	const struct kdbus_item *item;
 	bool policy_provided = false;
+	u64 attach_flags = 0;
 	int ret;
 
 	KDBUS_ITEMS_FOREACH(item, cmd->items, KDBUS_ITEMS_SIZE(cmd, items)) {
+
+		if (!KDBUS_ITEM_VALID(item, &cmd->items,
+				      KDBUS_ITEMS_SIZE(cmd, items)))
+			return -EINVAL;
+
 		switch (item->type) {
 		case KDBUS_ITEM_ATTACH_FLAGS:
-			conn->attach_flags = item->data64[0];
+			attach_flags = item->data64[0];
 			break;
 		case KDBUS_ITEM_NAME:
 		case KDBUS_ITEM_POLICY_ACCESS:
@@ -1798,6 +1804,11 @@ int kdbus_cmd_conn_update(struct kdbus_conn *conn,
 		}
 	}
 
+	if (!KDBUS_ITEMS_END(item, cmd->items, KDBUS_ITEMS_SIZE(cmd, items)))
+		return -EINVAL;
+
+	conn->attach_flags = attach_flags;
+
 	if (!policy_provided)
 		return 0;
 
@@ -1862,6 +1873,11 @@ int kdbus_conn_new(struct kdbus_ep *ep,
 
 	KDBUS_ITEMS_FOREACH(item, hello->items,
 			    KDBUS_ITEMS_SIZE(hello, items)) {
+
+		if (!KDBUS_ITEM_VALID(item, &hello->items,
+				      KDBUS_ITEMS_SIZE(hello, items)))
+			return -EINVAL;
+
 		switch (item->type) {
 		case KDBUS_ITEM_NAME:
 			if (!is_activator && !is_policy_holder)
@@ -1916,6 +1932,9 @@ int kdbus_conn_new(struct kdbus_ep *ep,
 		}
 	}
 
+	if (!KDBUS_ITEMS_END(item, hello->items, KDBUS_ITEMS_SIZE(hello, items)))
+		return -EINVAL;
+
 	if ((is_activator || is_policy_holder) && !name)
 		return -EINVAL;
 
-- 
1.9.0