From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751809AbdITJlA (ORCPT ); Wed, 20 Sep 2017 05:41:00 -0400 Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]:59392 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751606AbdITJk6 (ORCPT ); Wed, 20 Sep 2017 05:40:58 -0400 Date: Wed, 20 Sep 2017 10:41:09 +0100 From: Will Deacon To: viro@zeniv.linux.org.uk Cc: linux-kernel@vger.kernel.org, Andrew Morton Subject: Re: [PATCH] ipc/shm: Fix order of parameters when calling copy_compat_shmid_to_user Message-ID: <20170920094109.GC3782@arm.com> References: <1505753258-19470-1-git-send-email-will.deacon@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1505753258-19470-1-git-send-email-will.deacon@arm.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > Cc: Andrew Morton > Signed-off-by: Will Deacon > --- > 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 : [] lr : [] 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