| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 20 Dec 2016 16:36:36 +0800
From: Rui Xiang <rui.xiang@...wei.com>
To: Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
"H. Peter Anvin" <hpa@...or.com>, <x86@...nel.org>
CC: <linux-kernel@...r.kernel.org>, Jason Yan <yanaijie@...wei.com>
Subject: Re: [PATCH] x86/alternatives: add missing mutex lock
in,alternatives_text_reserved
Hi, all
In Sept 12, I reported a race between alternatives_text_reserved
and alternatives_smp_module_del. That also exists in mainline.
But adding mutex_lock(&smp_alt) in alternatives_text_reserved would
cause an obvious deadlock tested by Xiaolong <xiaolong.ye@...el.com>.
> [ 18.325461] Possible unsafe locking scenario:
> [ 18.325461]
> [ 18.329341] CPU0 CPU1
> [ 18.332070] ---- ----
> [ 18.335136] lock(text_mutex);
> [ 18.337314] lock(smp_alt);
> [ 18.340894] lock(text_mutex);
> [ 18.344641] lock(smp_alt);
> [ 18.346744]
> [ 18.346744] *** DEADLOCK ***
> [ 18.346744]
> [ 18.350563] 2 locks held by swapper/0/1:
> [ 18.353217] #0: (kprobe_mutex){+.+...}, at: [<ffffffffa914f331>] register_kprobe+0x171/0x720
> [ 18.358937] #1: (text_mutex){+.+...}, at: [<ffffffffa914f803>] register_kprobe+0x643/0x720
> [ 18.364580]
> [ 18.364580] stack backtrace:
> [ 18.367479] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.8.0-rc5-00438-g7c14957 #1
> [ 18.380205] ffffab5f80073bd0 ffffffffa953120a ffffffffaa8a1c40 ffffffffaa8a1c40
> [ 18.397397] ffffffffaa89d490 ffff99f10e8be820 ffffab5f80073c10 ffffffffa90e26d1
> [ 18.409510] 000000000e8be820 ffff99f10e8be858 ffff99f10e8be820 9b6c97af48d594d9
> [ 18.423190] Call Trace:
> [ 18.424696] [<ffffffffa953120a>] dump_stack+0xb3/0x119
> [ 18.432176] [<ffffffffa90e26d1>] print_circular_bug+0x201/0x220
> [ 18.440272] [<ffffffffa90e5fc9>] __lock_acquire+0xcd9/0x11c0
> [ 18.445740] [<ffffffffa9149d40>] ? stop_machine_from_inactive_cpu+0x160/0x160
> [ 18.450226] [<ffffffffa90e6891>] lock_acquire+0x71/0x90
> [ 18.453574] [<ffffffffa9026e7b>] ? alternatives_text_reserved+0x1b/0x90
> [ 18.470881] [<ffffffffa98dcbb1>] mutex_lock_nested+0x71/0x370
> [ 18.474546] [<ffffffffa9026e7b>] ? alternatives_text_reserved+0x1b/0x90
> [ 18.478720] [<ffffffffa90e3be2>] ? trace_hardirqs_on_caller+0x192/0x1d0
> [ 18.496043] [<ffffffffa9026e7b>] ? alternatives_text_reserved+0x1b/0x90
It may seem like a bit complicated to fix this race. And how to fix it,
what do you think?
Any advise is welcome.
Thanks,
Rui
On 2016/9/12 15:18, Rui Xiang wrote:
> A race between alternatives_text_reserved and alternatives_smp_module_del
> occured when traversing the global list smp_alt_modules in 3.4 kernel. It
> would casue a panic while reading smp_alt_modules->mod in
> alternatives_text_reserved func.
>
> [ 14.016190] general protection fault: 0000 [#1] SMP
> [ 14.016988] CPU 0
> [ 14.017287] Modules linked in: mlx4_ib(O+) mlx4_en(O+) xb_sa(O)
> [ 14.020005]
> [ 14.020005] Pid: 1979, comm: modprobe Tainted: G O
> [ 14.020005] RIP: 0010:[<ffffffff81007eb0>] [<ffffffff81007eb0>] alternatives_text_reserved+0x20/0x80
> [ 14.020005] RSP: 0018:ffff880ea355bcb8 EFLAGS: 00010283
> [ 14.020005] RAX: dead000000000100 RBX: ffffffffa02af720 RCX: dead0000000000d0
> [ 14.020005] RDX: ffffffffa02f0588 RSI: ffffffffa02d2fc0 RDI: ffffffffa02d2fc0
> [ 14.020005] RBP: ffff880ea355bcb8 R08: ffffffffa02f3b68 R09: 00017f4ae12d2fc0
> [ 14.020005] R10: 00000000000000e8 R11: ffffffffa02bb9d7 R12: 0000000000000000
> [ 14.020005] R13: ffffffffa02af720 R14: ffffffffa0307140 R15: ffffffffa02af730
> [ 14.020005] FS: 00007f26c6acc700(0000) GS:ffff880fff200000(0000) knlGS:0000000000000000
> [ 14.020005] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [ 14.020005] CR2: 00007fd4adc3b000 CR3: 0000000ea40ea000 CR4: 00000000001407f0
> [ 14.020005] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 14.020005] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [ 14.020005] Process modprobe (pid: 1979, threadinfo ffff880ea355a000, task ffff880e9eb9e600)
> [ 14.020005] Stack:
> [ 14.020005] ffff880ea355bcd8 ffffffff8145b148 ffffffffa02af720 ffffffffa02af720
> [ 14.020005] ffff880ea355bd18 ffffffff8145ed38 0000000000000000 0000000000000000
> [ 14.041015] ffff880ea355bd90 ffffffffa02af720 0000000000000001 ffff880ea355bd90
> [ 14.041015] Call Trace:
> [ 14.041015] [<ffffffff8145b148>] arch_prepare_kprobe+0x18/0x80
> [ 14.042982] [<ffffffff8145ed38>] register_kprobe+0x338/0x4c0
> [ 14.042982] [<ffffffff8145f658>] register_jprobes+0x98/0xc0
> [ 14.042982] [<ffffffff8145f69a>] register_jprobe+0x1a/0x20
>
> It also seems to exist in mainline.
>
> This patch adds the mutex lock smp_alt to fix it.
>
> Cc: stable@...r.kernel.org
> Signed-off-by: Rui Xiang <rui.xiang@...wei.com>
> ---
> arch/x86/kernel/alternative.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> index 5cb272a..7cec0cc 100644
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -563,6 +563,7 @@ int alternatives_text_reserved(void *start, void *end)
> u8 *text_start = start;
> u8 *text_end = end;
>
> + mutex_lock(&smp_alt);
> list_for_each_entry(mod, &smp_alt_modules, next) {
> if (mod->text > text_end || mod->text_end < text_start)
> continue;
> @@ -573,6 +574,7 @@ int alternatives_text_reserved(void *start, void *end)
> return 1;
> }
> }
> + mutex_unlock(&smp_alt);
>
> return 0;
> }
>
Powered by blists - more mailing lists