Re: [systemd-devel] [PATCH 1/2] connection: fix cpu stall by checking kdbus item alignment
On Wed, Jun 11, 2014 at 6:27 PM, Djalal Harouni wrote: > 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. Applied. Thanks, Kay ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/2] connection: fix cpu stall by checking kdbus item alignment
Hi,
The tests are not clean... just a copy/past if you want to confirm this
one! they are attached.
patch with policy-holder.patch
then run: test-kdbus-policy-holder
You should hit it!
On Wed, Jun 11, 2014 at 05:27:58PM +0100, Djalal Harouni wrote:
> 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: 88005a2719d0 ti: 88004587c000 task.ti:
> 88004587c000
> [ 142.731011] RIP: 0010:[] []
> flat_send_IPI_all+0x92/0xd0
> [ 142.731011] RSP: 0018:88005dc03dc0 EFLAGS: 00010006
> [ 142.731011] RAX: RBX: 0c00 RCX:
>
> [ 142.731011] RDX: 0c00 RSI: RDI:
> 0300
> [ 142.731011] RBP: 88005dc03dd8 R08: 0001 R09:
> 0001
> [ 142.731011] R10: 81e5d720 R11: 0001 R12:
> 0046
> [ 142.731011] R13: 000f R14: 81e86880 R15:
> 0003
> [ 142.731011] FS: 7f4b9d727740() GS:88005dc0()
> knlGS:
> [ 142.731011] CS: 0010 DS: ES: CR0: 8005003b
> [ 142.731011] CR2: 00401312 CR3: 5b66f000 CR4:
> 06e0
> [ 142.731011] Stack:
> [ 142.731011] 2710 81e86880 81fa3e40
> 88005dc03df0
> [ 142.731011] 81050654 88005ddcf000 88005dc03e48
> 8111a194
> [ 142.731011] 7fff 0046
> 10dc
> [ 142.731011] Call Trace:
> [ 142.731011]
>
> [ 142.731011] [] arch_trigger_all_cpu_backtrace+0x64/0xa0
> [ 142.731011] [] rcu_check_callbacks+0x584/0x850
> [ 142.731011] [] update_process_times+0x47/0x70
> [ 142.731011] [] tick_sched_handle.isra.19+0x25/0x60
> [ 142.731011] [] tick_sched_timer+0x41/0x60
> [ 142.731011] [] __run_hrtimer+0x86/0x460
> [ 142.731011] [] ? tick_sched_do_timer+0x40/0x40
> [ 142.731011] [] hrtimer_interrupt+0x10f/0x260
> [ 142.731011] [] local_apic_timer_interrupt+0x3a/0x60
> [ 142.731011] [] smp_apic_timer_interrupt+0x3f/0x50
> [ 142.731011] [] apic_timer_interrupt+0x72/0x80
> [ 142.731011]
>
> [ 142.731011] [] ? retint_restore_args+0x13/0x13
> [ 142.731011] [] ? kdbus_conn_new+0x25b/0xf20 [kdbus]
> [ 142.731011] [] ? kdbus_conn_new+0x331/0xf20 [kdbus]
> [ 142.731011] [] kdbus_handle_ioctl+0x221/0xad0 [kdbus]
> [ 142.731011] [] ? inode_has_perm.isra.47+0x51/0x90
> [ 142.731011] [] do_vfs_ioctl+0x2f0/0x520
> [ 142.731011] [] SyS_ioctl+0x81/0xa0
> [ 142.731011] [] system_call_fastpath+0x16/0x1b
>
> Signed-off-by: Djalal Harouni
> ---
> 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,
> +
[systemd-devel] [PATCH 1/2] connection: fix cpu stall by checking kdbus item alignment
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: 88005a2719d0 ti: 88004587c000 task.ti:
88004587c000
[ 142.731011] RIP: 0010:[] []
flat_send_IPI_all+0x92/0xd0
[ 142.731011] RSP: 0018:88005dc03dc0 EFLAGS: 00010006
[ 142.731011] RAX: RBX: 0c00 RCX:
[ 142.731011] RDX: 0c00 RSI: RDI: 0300
[ 142.731011] RBP: 88005dc03dd8 R08: 0001 R09: 0001
[ 142.731011] R10: 81e5d720 R11: 0001 R12: 0046
[ 142.731011] R13: 000f R14: 81e86880 R15: 0003
[ 142.731011] FS: 7f4b9d727740() GS:88005dc0()
knlGS:
[ 142.731011] CS: 0010 DS: ES: CR0: 8005003b
[ 142.731011] CR2: 00401312 CR3: 5b66f000 CR4: 06e0
[ 142.731011] Stack:
[ 142.731011] 2710 81e86880 81fa3e40
88005dc03df0
[ 142.731011] 81050654 88005ddcf000 88005dc03e48
8111a194
[ 142.731011] 7fff 0046
10dc
[ 142.731011] Call Trace:
[ 142.731011]
[ 142.731011] [] arch_trigger_all_cpu_backtrace+0x64/0xa0
[ 142.731011] [] rcu_check_callbacks+0x584/0x850
[ 142.731011] [] update_process_times+0x47/0x70
[ 142.731011] [] tick_sched_handle.isra.19+0x25/0x60
[ 142.731011] [] tick_sched_timer+0x41/0x60
[ 142.731011] [] __run_hrtimer+0x86/0x460
[ 142.731011] [] ? tick_sched_do_timer+0x40/0x40
[ 142.731011] [] hrtimer_interrupt+0x10f/0x260
[ 142.731011] [] local_apic_timer_interrupt+0x3a/0x60
[ 142.731011] [] smp_apic_timer_interrupt+0x3f/0x50
[ 142.731011] [] apic_timer_interrupt+0x72/0x80
[ 142.731011]
[ 142.731011] [] ? retint_restore_args+0x13/0x13
[ 142.731011] [] ? kdbus_conn_new+0x25b/0xf20 [kdbus]
[ 142.731011] [] ? kdbus_conn_new+0x331/0xf20 [kdbus]
[ 142.731011] [] kdbus_handle_ioctl+0x221/0xad0 [kdbus]
[ 142.731011] [] ? inode_has_perm.isra.47+0x51/0x90
[ 142.731011] [] do_vfs_ioctl+0x2f0/0x520
[ 142.731011] [] SyS_ioctl+0x81/0xa0
[ 142.731011] [] system_call_fastpath+0x16/0x1b
Signed-off-by: Djalal Harouni
---
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 -EINV
