* [PATCH] powerpc/futex: Fix incorrect user access blocking
@ 2020-02-07 12:21 Michael Ellerman
2020-02-07 12:30 ` Daniel Axtens
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Michael Ellerman @ 2020-02-07 12:21 UTC (permalink / raw)
To: linuxppc-dev; +Cc: dja
The early versions of our kernel user access prevention (KUAP) were
written by Russell and Christophe, and didn't have separate
read/write access.
At some point I picked up the series and added the read/write access,
but I failed to update the usages in futex.h to correctly allow read
and write.
However we didn't notice because of another bug which was causing the
low-level code to always enable read and write. That bug was fixed
recently in commit 1d8f739b07bd ("powerpc/kuap: Fix set direction in
allow/prevent_user_access()").
futex_atomic_cmpxchg_inatomic() is passed the user address as %3 and
does:
1: lwarx %1, 0, %3
cmpw 0, %1, %4
bne- 3f
2: stwcx. %5, 0, %3
Which clearly loads and stores from/to %3. The logic in
arch_futex_atomic_op_inuser() is similar, so fix both of them to use
allow_read_write_user().
Without this fix, and with PPC_KUAP_DEBUG=y, we see eg:
Bug: Read fault blocked by AMR!
WARNING: CPU: 94 PID: 149215 at arch/powerpc/include/asm/book3s/64/kup-radix.h:126 __do_page_fault+0x600/0xf30
CPU: 94 PID: 149215 Comm: futex_requeue_p Tainted: G W 5.5.0-rc7-gcc9x-g4c25df5640ae #1
...
NIP [c000000000070680] __do_page_fault+0x600/0xf30
LR [c00000000007067c] __do_page_fault+0x5fc/0xf30
Call Trace:
[c00020138e5637e0] [c00000000007067c] __do_page_fault+0x5fc/0xf30 (unreliable)
[c00020138e5638c0] [c00000000000ada8] handle_page_fault+0x10/0x30
--- interrupt: 301 at cmpxchg_futex_value_locked+0x68/0xd0
LR = futex_lock_pi_atomic+0xe0/0x1f0
[c00020138e563bc0] [c000000000217b50] futex_lock_pi_atomic+0x80/0x1f0 (unreliable)
[c00020138e563c30] [c00000000021b668] futex_requeue+0x438/0xb60
[c00020138e563d60] [c00000000021c6cc] do_futex+0x1ec/0x2b0
[c00020138e563d90] [c00000000021c8b8] sys_futex+0x128/0x200
[c00020138e563e20] [c00000000000b7ac] system_call+0x5c/0x68
Fixes: de78a9c42a79 ("powerpc: Add a framework for Kernel Userspace Access Protection")
Cc: stable@vger.kernel.org # v5.2+
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/futex.h | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/futex.h b/arch/powerpc/include/asm/futex.h
index eea28ca679db..bc7d9d06a6d9 100644
--- a/arch/powerpc/include/asm/futex.h
+++ b/arch/powerpc/include/asm/futex.h
@@ -35,7 +35,7 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
{
int oldval = 0, ret;
- allow_write_to_user(uaddr, sizeof(*uaddr));
+ allow_read_write_user(uaddr, uaddr, sizeof(*uaddr));
pagefault_disable();
switch (op) {
@@ -62,7 +62,7 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
*oval = oldval;
- prevent_write_to_user(uaddr, sizeof(*uaddr));
+ prevent_read_write_user(uaddr, uaddr, sizeof(*uaddr));
return ret;
}
@@ -76,7 +76,8 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
if (!access_ok(uaddr, sizeof(u32)))
return -EFAULT;
- allow_write_to_user(uaddr, sizeof(*uaddr));
+ allow_read_write_user(uaddr, uaddr, sizeof(*uaddr));
+
__asm__ __volatile__ (
PPC_ATOMIC_ENTRY_BARRIER
"1: lwarx %1,0,%3 # futex_atomic_cmpxchg_inatomic\n\
@@ -97,7 +98,8 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
: "cc", "memory");
*uval = prev;
- prevent_write_to_user(uaddr, sizeof(*uaddr));
+ prevent_read_write_user(uaddr, uaddr, sizeof(*uaddr));
+
return ret;
}
--
2.21.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] powerpc/futex: Fix incorrect user access blocking
2020-02-07 12:21 [PATCH] powerpc/futex: Fix incorrect user access blocking Michael Ellerman
@ 2020-02-07 12:30 ` Daniel Axtens
2020-02-07 16:03 ` Christophe Leroy
2020-02-08 12:50 ` Michael Ellerman
2 siblings, 0 replies; 4+ messages in thread
From: Daniel Axtens @ 2020-02-07 12:30 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
Michael Ellerman <mpe@ellerman.id.au> writes:
> The early versions of our kernel user access prevention (KUAP) were
> written by Russell and Christophe, and didn't have separate
> read/write access.
>
> At some point I picked up the series and added the read/write access,
> but I failed to update the usages in futex.h to correctly allow read
> and write.
>
> However we didn't notice because of another bug which was causing the
> low-level code to always enable read and write. That bug was fixed
> recently in commit 1d8f739b07bd ("powerpc/kuap: Fix set direction in
> allow/prevent_user_access()").
>
> futex_atomic_cmpxchg_inatomic() is passed the user address as %3 and
> does:
>
> 1: lwarx %1, 0, %3
> cmpw 0, %1, %4
> bne- 3f
> 2: stwcx. %5, 0, %3
>
> Which clearly loads and stores from/to %3. The logic in
> arch_futex_atomic_op_inuser() is similar, so fix both of them to use
> allow_read_write_user().
>
> Without this fix, and with PPC_KUAP_DEBUG=y, we see eg:
>
> Bug: Read fault blocked by AMR!
> WARNING: CPU: 94 PID: 149215 at arch/powerpc/include/asm/book3s/64/kup-radix.h:126 __do_page_fault+0x600/0xf30
> CPU: 94 PID: 149215 Comm: futex_requeue_p Tainted: G W 5.5.0-rc7-gcc9x-g4c25df5640ae #1
> ...
> NIP [c000000000070680] __do_page_fault+0x600/0xf30
> LR [c00000000007067c] __do_page_fault+0x5fc/0xf30
> Call Trace:
> [c00020138e5637e0] [c00000000007067c] __do_page_fault+0x5fc/0xf30 (unreliable)
> [c00020138e5638c0] [c00000000000ada8] handle_page_fault+0x10/0x30
> --- interrupt: 301 at cmpxchg_futex_value_locked+0x68/0xd0
> LR = futex_lock_pi_atomic+0xe0/0x1f0
> [c00020138e563bc0] [c000000000217b50] futex_lock_pi_atomic+0x80/0x1f0 (unreliable)
> [c00020138e563c30] [c00000000021b668] futex_requeue+0x438/0xb60
> [c00020138e563d60] [c00000000021c6cc] do_futex+0x1ec/0x2b0
> [c00020138e563d90] [c00000000021c8b8] sys_futex+0x128/0x200
> [c00020138e563e20] [c00000000000b7ac] system_call+0x5c/0x68
>
> Fixes: de78a9c42a79 ("powerpc: Add a framework for Kernel Userspace Access Protection")
Reported-by: syzbot+e808452bad7c375cbee6@syzkaller-ppc64.appspotmail.com
> Cc: stable@vger.kernel.org # v5.2+
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] powerpc/futex: Fix incorrect user access blocking
2020-02-07 12:21 [PATCH] powerpc/futex: Fix incorrect user access blocking Michael Ellerman
2020-02-07 12:30 ` Daniel Axtens
@ 2020-02-07 16:03 ` Christophe Leroy
2020-02-08 12:50 ` Michael Ellerman
2 siblings, 0 replies; 4+ messages in thread
From: Christophe Leroy @ 2020-02-07 16:03 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: dja
Le 07/02/2020 à 13:21, Michael Ellerman a écrit :
> The early versions of our kernel user access prevention (KUAP) were
> written by Russell and Christophe, and didn't have separate
> read/write access.
>
> At some point I picked up the series and added the read/write access,
> but I failed to update the usages in futex.h to correctly allow read
> and write.
>
> However we didn't notice because of another bug which was causing the
> low-level code to always enable read and write. That bug was fixed
> recently in commit 1d8f739b07bd ("powerpc/kuap: Fix set direction in
> allow/prevent_user_access()").
>
> futex_atomic_cmpxchg_inatomic() is passed the user address as %3 and
> does:
>
> 1: lwarx %1, 0, %3
> cmpw 0, %1, %4
> bne- 3f
> 2: stwcx. %5, 0, %3
>
> Which clearly loads and stores from/to %3. The logic in
> arch_futex_atomic_op_inuser() is similar, so fix both of them to use
> allow_read_write_user().
>
> Without this fix, and with PPC_KUAP_DEBUG=y, we see eg:
>
> Bug: Read fault blocked by AMR!
> WARNING: CPU: 94 PID: 149215 at arch/powerpc/include/asm/book3s/64/kup-radix.h:126 __do_page_fault+0x600/0xf30
> CPU: 94 PID: 149215 Comm: futex_requeue_p Tainted: G W 5.5.0-rc7-gcc9x-g4c25df5640ae #1
> ...
> NIP [c000000000070680] __do_page_fault+0x600/0xf30
> LR [c00000000007067c] __do_page_fault+0x5fc/0xf30
> Call Trace:
> [c00020138e5637e0] [c00000000007067c] __do_page_fault+0x5fc/0xf30 (unreliable)
> [c00020138e5638c0] [c00000000000ada8] handle_page_fault+0x10/0x30
> --- interrupt: 301 at cmpxchg_futex_value_locked+0x68/0xd0
> LR = futex_lock_pi_atomic+0xe0/0x1f0
> [c00020138e563bc0] [c000000000217b50] futex_lock_pi_atomic+0x80/0x1f0 (unreliable)
> [c00020138e563c30] [c00000000021b668] futex_requeue+0x438/0xb60
> [c00020138e563d60] [c00000000021c6cc] do_futex+0x1ec/0x2b0
> [c00020138e563d90] [c00000000021c8b8] sys_futex+0x128/0x200
> [c00020138e563e20] [c00000000000b7ac] system_call+0x5c/0x68
>
> Fixes: de78a9c42a79 ("powerpc: Add a framework for Kernel Userspace Access Protection")
> Cc: stable@vger.kernel.org # v5.2+
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/include/asm/futex.h | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/futex.h b/arch/powerpc/include/asm/futex.h
> index eea28ca679db..bc7d9d06a6d9 100644
> --- a/arch/powerpc/include/asm/futex.h
> +++ b/arch/powerpc/include/asm/futex.h
> @@ -35,7 +35,7 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
> {
> int oldval = 0, ret;
>
> - allow_write_to_user(uaddr, sizeof(*uaddr));
> + allow_read_write_user(uaddr, uaddr, sizeof(*uaddr));
> pagefault_disable();
>
> switch (op) {
> @@ -62,7 +62,7 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
>
> *oval = oldval;
>
> - prevent_write_to_user(uaddr, sizeof(*uaddr));
> + prevent_read_write_user(uaddr, uaddr, sizeof(*uaddr));
> return ret;
> }
>
> @@ -76,7 +76,8 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
> if (!access_ok(uaddr, sizeof(u32)))
> return -EFAULT;
>
> - allow_write_to_user(uaddr, sizeof(*uaddr));
> + allow_read_write_user(uaddr, uaddr, sizeof(*uaddr));
> +
> __asm__ __volatile__ (
> PPC_ATOMIC_ENTRY_BARRIER
> "1: lwarx %1,0,%3 # futex_atomic_cmpxchg_inatomic\n\
> @@ -97,7 +98,8 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
> : "cc", "memory");
>
> *uval = prev;
> - prevent_write_to_user(uaddr, sizeof(*uaddr));
> + prevent_read_write_user(uaddr, uaddr, sizeof(*uaddr));
> +
> return ret;
> }
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] powerpc/futex: Fix incorrect user access blocking
2020-02-07 12:21 [PATCH] powerpc/futex: Fix incorrect user access blocking Michael Ellerman
2020-02-07 12:30 ` Daniel Axtens
2020-02-07 16:03 ` Christophe Leroy
@ 2020-02-08 12:50 ` Michael Ellerman
2 siblings, 0 replies; 4+ messages in thread
From: Michael Ellerman @ 2020-02-08 12:50 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: dja
On Fri, 2020-02-07 at 12:21:45 UTC, Michael Ellerman wrote:
> The early versions of our kernel user access prevention (KUAP) were
> written by Russell and Christophe, and didn't have separate
> read/write access.
>
> At some point I picked up the series and added the read/write access,
> but I failed to update the usages in futex.h to correctly allow read
> and write.
>
> However we didn't notice because of another bug which was causing the
> low-level code to always enable read and write. That bug was fixed
> recently in commit 1d8f739b07bd ("powerpc/kuap: Fix set direction in
> allow/prevent_user_access()").
>
> futex_atomic_cmpxchg_inatomic() is passed the user address as %3 and
> does:
>
> 1: lwarx %1, 0, %3
> cmpw 0, %1, %4
> bne- 3f
> 2: stwcx. %5, 0, %3
>
> Which clearly loads and stores from/to %3. The logic in
> arch_futex_atomic_op_inuser() is similar, so fix both of them to use
> allow_read_write_user().
>
> Without this fix, and with PPC_KUAP_DEBUG=y, we see eg:
>
> Bug: Read fault blocked by AMR!
> WARNING: CPU: 94 PID: 149215 at arch/powerpc/include/asm/book3s/64/kup-radix.h:126 __do_page_fault+0x600/0xf30
> CPU: 94 PID: 149215 Comm: futex_requeue_p Tainted: G W 5.5.0-rc7-gcc9x-g4c25df5640ae #1
> ...
> NIP [c000000000070680] __do_page_fault+0x600/0xf30
> LR [c00000000007067c] __do_page_fault+0x5fc/0xf30
> Call Trace:
> [c00020138e5637e0] [c00000000007067c] __do_page_fault+0x5fc/0xf30 (unreliable)
> [c00020138e5638c0] [c00000000000ada8] handle_page_fault+0x10/0x30
> --- interrupt: 301 at cmpxchg_futex_value_locked+0x68/0xd0
> LR = futex_lock_pi_atomic+0xe0/0x1f0
> [c00020138e563bc0] [c000000000217b50] futex_lock_pi_atomic+0x80/0x1f0 (unreliable)
> [c00020138e563c30] [c00000000021b668] futex_requeue+0x438/0xb60
> [c00020138e563d60] [c00000000021c6cc] do_futex+0x1ec/0x2b0
> [c00020138e563d90] [c00000000021c8b8] sys_futex+0x128/0x200
> [c00020138e563e20] [c00000000000b7ac] system_call+0x5c/0x68
>
> Fixes: de78a9c42a79 ("powerpc: Add a framework for Kernel Userspace Access Protection")
> Cc: stable@vger.kernel.org # v5.2+
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Applied to powerpc fixes.
https://git.kernel.org/powerpc/c/9dc086f1e9ef39dd823bd27954b884b2062f9e70
cheers
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-02-08 12:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-07 12:21 [PATCH] powerpc/futex: Fix incorrect user access blocking Michael Ellerman
2020-02-07 12:30 ` Daniel Axtens
2020-02-07 16:03 ` Christophe Leroy
2020-02-08 12:50 ` Michael Ellerman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.