[PATCH] futex:fix robust futex alignment exception

[PATCH] futex:fix robust futex alignment exception

[chenjie6]

From: chen jie <chenjie6@huawei.com>

trinity test bug fix:
/tmp/trinity --children 4 --quiet -N 10000000 --logging=off -X -x perf_event_open --enable-fds=testfile

[1542.195981] Task track: trinity-c3(6911)>trinity-main(28313)>sh(839)>bash(824)>sshd(820)>sshd(662)>init(1)
[11542.214694] Alignment trap: not handling instruction e1915f9f at [<c017b1d4>]
[11542.214724] Unhandled fault: alignment exception (0x011) at 0x000265f9
[11542.214749] pgd = edde0000
[11542.214774] [000265f9] *pgd=84aa9831, *pte=bc10359f, *ppte=bc103e7e
[11542.214851] Internal error: : 11 [#1] SMP ARM
[11542.214857] Modules linked in: rtos_snapshot(O) rsm(O) nfsv3 veth(O) pthread_lsof(O) higmac(O) comm(O) nand mtdblock mtd_blkdevs nand_ecc nand_ids pramdisk(O) rtos_kbox_panic(O) double_cluster(O) uart_suspend(O) cache_ops(O) nfsd nfs_acl exportfs auth_rpcgss nfs lockd sunrpc oid_registry grace physmap cfi_probe cfi_cmdset_0002 cfi_util mtd gen_probe chipreg ohci_platform ehci_platform ohci_hcd ehci_hcd usb_device_hisi(O) vfat fat sd_mod enable_uart_rx(O) [last unloaded: rtos_snapshot]
[11542.215042] CPU: 3 PID: 6911 Comm: trinity-c3 Tainted: G    B   W  O    4.1.12 #1
[11542.215048] Hardware name: Hisilicon A9
[11542.215055] task: c3df8a20 ti: ebb2c000 task.ti: ebb2c000
[11542.215071] PC is at cmpxchg_futex_value_locked+0x44/0x88
[11542.215081] LR is at handle_futex_death+0x78/0xcc
[11542.215090] pc : [<c017b1d4>]    lr : [<c017da50>]    psr: 60000213
sp : ebb2dee4  ip : fffffff2  fp : fffffff2
[11542.215096] r10: 000238e3  r9 : 00000000  r8 : 00001000
[11542.215103] r7 : c3df8a20  r6 : 00000000  r5 : 00001aff  r4 : ebb2def4
[11542.215110] r3 : 40000000  r2 : 00001aff  r1 : 000265f9  r0 : 410265fc
[11542.215119] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
[11542.215126] Control: 1ac5387d  Table: ae7e004a  DAC: 55555555
[11542.215133] Process trinity-c3 (pid: 6911, stack limit = 0xebb2c210)
[11542.215140] Stack: (0xebb2dee4 to 0xebb2e000)
[11542.215151] dee0:          000265f9 00001aff c017da50 000265f9 c3df8a20 b5ebc000 00000800
[11542.215161] df00: c3df8a20 00001000 00001000 c017dba8 c3df8a20 c399ef40 00000000 c3df8a20
[11542.215172] df20: c399ef40 c399ef40 000000f8 c0107b84 ebb2c000 00000001 0094d810 c011b40c
[11542.215182] df40: c3df8a20 c399ef40 c3df8a20 c399ef40 0094d830 c011f9a4 00000000 000000f8
[11542.215192] df60: c0107b84 c0197388 00002d16 ef1d3520 00000000 0094d830 000000f8 c0107b84
[11542.215203] df80: ebb2c000 00000200 0094d810 c0120250 00097d80 0094d8a4 0094d830 c01202a8
[11542.215213] dfa0: 00000000 c0107b6c 00097d80 0094d8a4 00000000 b6f0f4c0 b63ef000 00000000
[11542.215223] dfc0: 00097d80 0094d8a4 0094d830 000000f8 00000001 0094db88 0094db94 0094d810
[11542.215233] dfe0: 00097d64 be938310 00017a40 b6e1a340 60000210 00000000 00000000 00000000
[11542.215247] [<c017b1d4>] (cmpxchg_futex_value_locked) from [<c017da50>] (handle_futex_death+0x78/0xcc)
[11542.215259] [<c017da50>] (handle_futex_death) from [<c017dba8>] (exit_robust_list+0x104/0x160)
[11542.215273] [<c017dba8>] (exit_robust_list) from [<c011b40c>] (mm_release+0x1c/0x108)
[11542.215287] [<c011b40c>] (mm_release) from [<c011f9a4>] (do_exit+0x218/0x9a4)
[11542.215299] [<c011f9a4>] (do_exit) from [<c0120250>] (do_group_exit+0xac/0xf4)
[11542.215311] [<c0120250>] (do_group_exit) from [<c01202a8>] (__wake_up_parent+0x0/0x18)
[11542.215321] Code: 0dc0e0e3 0a00001a 5bf07ff5 00f091f5 (9f5f91e1)
[11542.217918] CPU 1 will stop doing anything useful since another CPU has crashed
[11542.217924] CPU 0 will stop doing anything useful since another CPU has crashed
[11542.217930] CPU 2 will stop doing anything useful since another CPU has crashed
[11542.218626] Loading crashdump kernel...
[11542.218668] Bye!

Signed-off-by: chen jie <chenjie6@huawei.com>
---
 kernel/futex.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/futex.c b/kernel/futex.c
index a0514e0..70231c4 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -3440,6 +3440,9 @@ static int handle_futex_death(u32 __user *uaddr, struct task_struct *curr, int p
 {
 	u32 uval, uninitialized_var(nval), mval;
 
+	if (((unsigned long)uaddr & 0x3) > 0)
+		return -1;
+
 retry:
 	if (get_user(uval, uaddr))
 		return -1;
-- 
1.8.3.4

Re: [PATCH] futex:fix robust futex alignment exception

[Peter Zijlstra]

On Fri, Mar 15, 2019 at 03:44:38AM +0000, chenjie6@huawei.com wrote:
> From: chen jie <chenjie6@huawei.com>

> [11542.215247] [<c017b1d4>] (cmpxchg_futex_value_locked) from [<c017da50>] (handle_futex_death+0x78/0xcc)
> [11542.215259] [<c017da50>] (handle_futex_death) from [<c017dba8>] (exit_robust_list+0x104/0x160)
> [11542.215273] [<c017dba8>] (exit_robust_list) from [<c011b40c>] (mm_release+0x1c/0x108)
> [11542.215287] [<c011b40c>] (mm_release) from [<c011f9a4>] (do_exit+0x218/0x9a4)
> [11542.215299] [<c011f9a4>] (do_exit) from [<c0120250>] (do_group_exit+0xac/0xf4)
> [11542.215311] [<c0120250>] (do_group_exit) from [<c01202a8>] (__wake_up_parent+0x0/0x18)

> Signed-off-by: chen jie <chenjie6@huawei.com>

Reviewed-by: Peter Zijlstra (Intel) <peterz@infradead.org>

However, should there not also be alignment tests on set_robust_list()?

Also; do_futex() should probably check uaddr and uaddr2.

That is; why aren't there any alignment tests anywhere? Or am I just
gone blind?

> ---
>  kernel/futex.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/kernel/futex.c b/kernel/futex.c
> index a0514e0..70231c4 100644
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -3440,6 +3440,9 @@ static int handle_futex_death(u32 __user *uaddr, struct task_struct *curr, int p
>  {
>  	u32 uval, uninitialized_var(nval), mval;
>  
> +	if (((unsigned long)uaddr & 0x3) > 0)
> +		return -1;
> +
>  retry:
>  	if (get_user(uval, uaddr))
>  		return -1;
> -- 
> 1.8.3.4
> 

Re: [PATCH] futex:fix robust futex alignment exception

[Thomas Gleixner]

On Fri, 15 Mar 2019, Peter Zijlstra wrote:

> On Fri, Mar 15, 2019 at 03:44:38AM +0000, chenjie6@huawei.com wrote:
> > From: chen jie <chenjie6@huawei.com>
> 
> > [11542.215247] [<c017b1d4>] (cmpxchg_futex_value_locked) from [<c017da50>] (handle_futex_death+0x78/0xcc)
> > [11542.215259] [<c017da50>] (handle_futex_death) from [<c017dba8>] (exit_robust_list+0x104/0x160)
> > [11542.215273] [<c017dba8>] (exit_robust_list) from [<c011b40c>] (mm_release+0x1c/0x108)
> > [11542.215287] [<c011b40c>] (mm_release) from [<c011f9a4>] (do_exit+0x218/0x9a4)
> > [11542.215299] [<c011f9a4>] (do_exit) from [<c0120250>] (do_group_exit+0xac/0xf4)
> > [11542.215311] [<c0120250>] (do_group_exit) from [<c01202a8>] (__wake_up_parent+0x0/0x18)
> 
> > Signed-off-by: chen jie <chenjie6@huawei.com>
> 
> Reviewed-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> 
> However, should there not also be alignment tests on set_robust_list()?
> 
> Also; do_futex() should probably check uaddr and uaddr2.
> 
> That is; why aren't there any alignment tests anywhere? Or am I just
> gone blind?

uaddrs for the futex syscalls are checked in get_futex_key().

Thanks,

	tglx

Re: [PATCH] futex:fix robust futex alignment exception

[Peter Zijlstra]

On Sun, Mar 17, 2019 at 03:36:35PM +0100, Thomas Gleixner wrote:
> On Fri, 15 Mar 2019, Peter Zijlstra wrote:

> > That is; why aren't there any alignment tests anywhere? Or am I just
> > gone blind?
> 
> uaddrs for the futex syscalls are checked in get_futex_key().

blind it is...

Thanks!

Re: [PATCH] futex:fix robust futex alignment exception

[Chenjie (K)]

The test case:

#include <stdio.h>
#include <linux/futex.h>
#include <syscall.h>
#include <unistd.h>
  #include <stdlib.h>

int main()
{
	char *p = malloc(128);

	struct robust_list_head *ro1;
	struct robust_list *entry;
	struct robust_list *pending;

	int ret = 0;

	pid_t pid = getpid();
	
	printf("p %p  pid [%d] \n", p, pid);

	ro1 = p;
	entry = p + 20;
	pending = p + 40;

	ro1->list.next = entry;
	ro1->list_op_pending = pending;

	entry->next = &(ro1->list);

	ro1->futex_offset = 41;

	*((int *)((char *)entry + 41)) = pid;

	printf(" entry + offert [%p] [%d] \n",  (int *)((char *)entry + 41), 
*((int *)((char *)entry + 41)));

	ret = syscall(SYS_set_robust_list, ro1, 12);

	printf("ret = [%d]\n", ret);


	return 0;
}

and we test it on arm a9 platform

Alignment trap: not handling instruction e191ef9f at [<c018cf5c>]
Unhandled fault: alignment exception (0x011) at 0x01b1218d
pgd = c3b50000
[01b1218d] *pgd=843c8831, *pte=b831d75f, *ppte=b831dc7f
Internal error: : 11 [#1] SMP ARM
Modules linked in: nfsv3 veth(O) ping(O) nand mtdblock mtd_blkdevs 
nand_ecc nand_ids gmac(O) pramdisk(O) rtos_kbox_panic(O) 
rtos_snapshot(O) double_cluster(O) uart_suspend(O) rsm(O) 
follow_huge_pfn(O) cache_ops(O) nfsd auth_rpcgss exportfs nfs_acl nfs 
lockd sunrpc oid_registry grace physmap cfi_cmdset_0002 cfi_probe 
cfi_util mtd gen_probe chipreg ohci_platform ehci_platform ohci_hcd 
ehci_hcd vfat fat sd_mod enable_uart_rx(O)
CPU: 1 PID: 786 Comm: set_robust_list Tainted: G        W  O    4.4.171 #3
Hardware name: Hisilicon A9
task: ef0045e8 task.stack: c3b68000
PC is at cmpxchg_futex_value_locked+0x48/0xac
LR is at 0x42b12190
pc : [<c018cf5c>]    lr : [<42b12190>]    psr: 60070213
sp : c3b69ed8  ip : fffffff2  fp : c05d9eeb
r10: ffffe000  r9 : ef0045e8  r8 : 00000000
r7 : 01b12178  r6 : 00000312  r5 : 01b1218d  r4 : ef0045e8
r3 : 40000000  r2 : 00000312  r1 : 01b1218d  r0 : c3b69ee0
Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
Control: 1ac5387d  Table: 8455004a  DAC: 55555555
Process set_robust_list (pid: 786, stack limit = 0xc3b68210)
Stack: (0xc3b69ed8 to 0xc3b6a000)
9ec0:                                                       c080e2a8 
c018ff44
9ee0: 01b1218d dc8ba6ab 00000000 01b12164 01b12150 ef0045e8 01b12178 
01b12150
9f00: 00000000 00000029 01b12178 c01900dc 00000800 00000000 ffffe000 
00000000
9f20: ffffe000 ef0045e8 c2e6dc00 ffffe000 c2e6dc00 c01077a4 00000000 
000000f8
9f40: b6fa2e30 c011afd8 ef0045e8 c2e6dc00 dc8ba6ab ef0045e8 c2e6dc00 
ffffe000
9f60: ffffe000 c011f370 00000000 ef00486c ef00486c dc8ba6ab c38aed20 
ffffe000
9f80: b6fa2e30 c0120d44 00000000 b6fa1500 00000000 000000f8 c01077a4 
c0120e14
9fa0: b6fa1500 c0107790 b6fa1500 b6fa1500 00000000 00000000 0097fadd 
00000000
9fc0: b6fa1500 b6fa1500 00000000 000000f8 00000000 00000001 b6fa6120 
b6fa2e30
9fe0: 00000000 be80eb98 b6e8d4c8 b6efe53c 60070210 00000000 00000000 
00000000
[<c018cf5c>] (cmpxchg_futex_value_locked) from [<c018ff44>] 
(handle_futex_death+0xa8/0x110)
[<c018ff44>] (handle_futex_death) from [<c01900dc>] 
(exit_robust_list+0x130/0x1b4)
[<c01900dc>] (exit_robust_list) from [<c011afd8>] (mm_release+0x1c/0x13c)
[<c011afd8>] (mm_release) from [<c011f370>] (do_exit+0x240/0x9b8)
[<c011f370>] (do_exit) from [<c0120d44>] (do_group_exit+0x58/0x108)
[<c0120d44>] (do_group_exit) from [<c0120e14> (__wake_up_parent+0x0/0x18)
Code: 0c40a011 0900001a 5bf07ff5 00f091f5 (9fef91e1)


On 2019/3/18 18:48, Peter Zijlstra wrote:
> On Sun, Mar 17, 2019 at 03:36:35PM +0100, Thomas Gleixner wrote:
>> On Fri, 15 Mar 2019, Peter Zijlstra wrote:
>
>>> That is; why aren't there any alignment tests anywhere? Or am I just
>>> gone blind?
>>
>> uaddrs for the futex syscalls are checked in get_futex_key().
>
> blind it is...
>
> Thanks!
>
>

[tip:locking/urgent] futex: Ensure that futex address is aligned in handle_futex_death()

[tip-bot for Chen Jie]

Commit-ID:  5a07168d8d89b00fe1760120714378175b3ef992
Gitweb:     https://git.kernel.org/tip/5a07168d8d89b00fe1760120714378175b3ef992
Author:     Chen Jie <chenjie6@huawei.com>
AuthorDate: Fri, 15 Mar 2019 03:44:38 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 22 Mar 2019 13:05:26 +0100

futex: Ensure that futex address is aligned in handle_futex_death()

The futex code requires that the user space addresses of futexes are 32bit
aligned. sys_futex() checks this in futex_get_keys() but the robust list
code has no alignment check in place.

As a consequence the kernel crashes on architectures with strict alignment
requirements in handle_futex_death() when trying to cmpxchg() on an
unaligned futex address which was retrieved from the robust list.

[ tglx: Rewrote changelog, proper sizeof() based alignement check and add
  	comment ]

Fixes: 0771dfefc9e5 ("[PATCH] lightweight robust futexes: core")
Signed-off-by: Chen Jie <chenjie6@huawei.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: <dvhart@infradead.org>
Cc: <peterz@infradead.org>
Cc: <zengweilin@huawei.com>
Cc: stable@vger.kernel.org
Link: https://lkml.kernel.org/r/1552621478-119787-1-git-send-email-chenjie6@huawei.com

---
 kernel/futex.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/futex.c b/kernel/futex.c
index c3b73b0311bc..9e40cf7be606 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -3436,6 +3436,10 @@ static int handle_futex_death(u32 __user *uaddr, struct task_struct *curr, int p
 {
 	u32 uval, uninitialized_var(nval), mval;
 
+	/* Futex address must be 32bit aligned */
+	if ((((unsigned long)uaddr) % sizeof(*uaddr)) != 0)
+		return -1;
+
 retry:
 	if (get_user(uval, uaddr))
 		return -1;