* Re: [PATCH] ipc/shm: Fix order of parameters when calling copy_compat_shmid_to_user
2017-09-18 16:47 [PATCH] ipc/shm: Fix order of parameters when calling copy_compat_shmid_to_user Will Deacon
@ 2017-09-20 9:41 ` Will Deacon
2017-09-20 11:01 ` Al Viro
1 sibling, 0 replies; 3+ messages in thread
From: Will Deacon @ 2017-09-20 9:41 UTC (permalink / raw)
To: viro; +Cc: linux-kernel, Andrew Morton
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
^ permalink raw reply [flat|nested] 3+ messages in thread