Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1734298 > unrolled thread
| Started by | Will Deacon <will.deacon@arm.com> |
|---|---|
| First post | 2017-09-18 18:50 +0200 |
| Last post | 2017-09-20 13:10 +0200 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH] ipc/shm: Fix order of parameters when calling copy_compat_shmid_to_user Will Deacon <will.deacon@arm.com> - 2017-09-18 18:50 +0200
Re: [PATCH] ipc/shm: Fix order of parameters when calling copy_compat_shmid_to_user Will Deacon <will.deacon@arm.com> - 2017-09-20 11:50 +0200
Re: [PATCH] ipc/shm: Fix order of parameters when calling copy_compat_shmid_to_user Al Viro <viro@ZenIV.linux.org.uk> - 2017-09-20 13:10 +0200
| From | Will Deacon <will.deacon@arm.com> |
|---|---|
| Date | 2017-09-18 18:50 +0200 |
| Subject | [PATCH] ipc/shm: Fix order of parameters when calling copy_compat_shmid_to_user |
| Message-ID | <ur7p8-ju-13@gated-at.bofh.it> |
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;
--
2.1.4
[toc] | [next] | [standalone]
| From | Will Deacon <will.deacon@arm.com> |
|---|---|
| Date | 2017-09-20 11:50 +0200 |
| Subject | Re: [PATCH] ipc/shm: Fix order of parameters when calling copy_compat_shmid_to_user |
| Message-ID | <urJNN-1dj-25@gated-at.bofh.it> |
| In reply to | #1734298 |
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
[toc] | [prev] | [next] | [standalone]
| From | Al Viro <viro@ZenIV.linux.org.uk> |
|---|---|
| Date | 2017-09-20 13:10 +0200 |
| Subject | Re: [PATCH] ipc/shm: Fix order of parameters when calling copy_compat_shmid_to_user |
| Message-ID | <urL3b-2f1-3@gated-at.bofh.it> |
| In reply to | #1734298 |
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.
ACK, will push to Linus tonight...
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web