Re: [PATCH] ipc/shm: Fix order of parameters when calling copy_compat_shmid_to_user

[Will Deacon <will.deacon@arm.com>]

On Mon, Sep 18, 2017 at 05:47:38PM +0100, Will Deacon wrote:
> Commit 553f770ef71b ("ipc: move compat shmctl to native") moved the
> compat IPC syscall handling into ipc/shm.c and refactored the struct
> accessors in the process. Unfortunately, the call to
> copy_compat_shmid_to_user when handling a compat {IPC,SHM}_STAT command
> gets the arguments the wrong way round, passing a kernel stack address
> as the user buffer (destination) and the user buffer as the kernel stack
> address (source).
> 
> This patch fixes the parameter ordering so the buffers are accessed
> correctly.
> 
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  ipc/shm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ipc/shm.c b/ipc/shm.c
> index 1b3adfe3c60e..1e2b1692ba2c 100644
> --- a/ipc/shm.c
> +++ b/ipc/shm.c
> @@ -1237,7 +1237,7 @@ COMPAT_SYSCALL_DEFINE3(shmctl, int, shmid, int, cmd, void __user *, uptr)
>  		err = shmctl_stat(ns, shmid, cmd, &sem64);
>  		if (err < 0)
>  			return err;
> -		if (copy_compat_shmid_to_user(&sem64, uptr, version))
> +		if (copy_compat_shmid_to_user(uptr, &sem64, version))
>  			err = -EFAULT;
>  		return err;

FWIW, this can easily Oops an arm64 kernel with a 32-bit userspace. LTP's
hugeshmctl02 test tickles the problem by doing:

	[...]
	shmget(0x710e4ddf, 134217728, IPC_CREAT|IPC_EXCL|SHM_HUGETLB|0600) = 32769
	[...]
	shmctl(32769, IPC_64|IPC_STAT, 0xffffffff)

which causes:

[  345.655736] Unable to handle kernel paging request at virtual address ffffffff
[  345.677173] Mem abort info:
[  345.685444]   Exception class = DABT (current EL), IL = 32 bits
[  345.702981]   SET = 0, FnV = 0
[  345.712016]   EA = 0, S1PTW = 0
[  345.721308] Data abort info:
[  345.729838]   ISV = 0, ISS = 0x00000006
[  345.741189]   CM = 0, WnR = 0
[  345.749966] user pgtable: 4k pages, 48-bit VAs, pgd = ffff800975801000
[  345.769306] [00000000ffffffff] *pgd=00000009f5397003, *pud=00000009f4c16003, *pmd=0000000000000000
[  345.795873] Internal error: Oops: 96000006 [#1] PREEMPT SMP
[  345.812370] Modules linked in:
[  345.821400] CPU: 2 PID: 2586 Comm: hugeshmctl02 Not tainted 4.14.0-rc1 #1
[  345.841506] Hardware name: ARM Juno development board (r2) (DT)
[  345.795873] Internal error: Oops: 96000006 [#1] PREEMPT SMP
[  345.894072] task: ffff8009758bf000 task.stack: ffff00000dc70000
[  345.911607] PC is at to_compat_ipc64_perm+0x0/0x40
[  345.925789] LR is at copy_compat_shmid_to_user+0xbc/0x120
[  345.941770] pc : [<ffff000008362350>] lr : [<ffff000008368a2c>] pstate: 60000145
[  345.963678] sp : ffff00000dc73d60
[  345.973475] x29: ffff00000dc73d60 x28: ffff8009758bf000 
[  345.989203] x27: ffff0000089d4000 x26: 0000000000000134 
[  346.004930] x25: 000000000000018e x24: ffff000008f52eb0 
[  346.020656] x23: 00000000ffffffff x22: ffff8009758bf000 
[  346.036383] x21: 0000000000000100 x20: ffff00000dc73e50 
[  346.052109] x19: 00000000ffffffff x18: 0000000000000000 
[  346.067836] x17: 0000000000000000 x16: ffff000008369cd8 
[  346.083562] x15: 0000000000000000 x14: 00000000f7a2872f 
[  346.099289] x13: 00000000ffdc9334 x12: 0000000000000134 
[  346.115015] x11: 000000000002eeb4 x10: 0000000000000040 
[  346.130742] x9 : ffff000008f530a8 x8 : ffff800976e29240 
[  346.146468] x7 : ffff800976e29268 x6 : ffff000008f3fa48 
[  346.162195] x5 : 0000000000000001 x4 : 0000000000000000 
[  346.177921] x3 : ffff000008f3fa48 x2 : 0000000000000100 
[  346.193648] x1 : 00000000ffffffff x0 : ffff00000dc73d88 
[  346.209375] Process hugeshmctl02 (pid: 2586, stack limit = 0xffff00000dc70000)

Will