| Message ID | 1505753258-19470-1-git-send-email-will.deacon@arm.com |
|---|---|
| State | Accepted |
| Commit | 58aff0af757356065f33290d96a9cd46dfbcae88 |
| Headers | show |
| Series | ipc/shm: Fix order of parameters when calling copy_compat_shmid_to_user | expand |
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
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...
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;
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(-) -- 2.1.4