[v4.9.y,10/42] arm64: futex: Mask __user pointers prior to dereference

Message ID 20180412111138.40990-11-mark.rutland@arm.com
State New
Headers show
Series
  • arm64 spectre patches
Related show

Commit Message

Mark Rutland April 12, 2018, 11:11 a.m.
From: Will Deacon <will.deacon@arm.com>


commit 91b2d3442f6a44dce875670d702af22737ad5eff upstream.

The arm64 futex code has some explicit dereferencing of user pointers
where performing atomic operations in response to a futex command. This
patch uses masking to limit any speculative futex operations to within
the user address space.

Signed-off-by: Will Deacon <will.deacon@arm.com>

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>

Signed-off-by: Mark Rutland <mark.rutland@arm.com> [v4.9 backport]

---
 arch/arm64/include/asm/futex.h | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

-- 
2.11.0

Comments

Greg KH April 17, 2018, 12:10 p.m. | #1
On Thu, Apr 12, 2018 at 12:11:06PM +0100, Mark Rutland wrote:
> From: Will Deacon <will.deacon@arm.com>

> 

> commit 91b2d3442f6a44dce875670d702af22737ad5eff upstream.

> 

> The arm64 futex code has some explicit dereferencing of user pointers

> where performing atomic operations in response to a futex command. This

> patch uses masking to limit any speculative futex operations to within

> the user address space.

> 

> Signed-off-by: Will Deacon <will.deacon@arm.com>

> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>

> Signed-off-by: Mark Rutland <mark.rutland@arm.com> [v4.9 backport]

> Tested-by: Greg Hackmann <ghackmann@google.com>

> ---

>  arch/arm64/include/asm/futex.h | 9 ++++++---

>  1 file changed, 6 insertions(+), 3 deletions(-)

> 

> diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h

> index f2585cdd32c2..1d123dd01ee0 100644

> --- a/arch/arm64/include/asm/futex.h

> +++ b/arch/arm64/include/asm/futex.h

> @@ -51,13 +51,14 @@

>  	: "memory")

>  

>  static inline int

> -futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)

> +futex_atomic_op_inuser (int encoded_op, u32 __user *_uaddr)

>  {

>  	int op = (encoded_op >> 28) & 7;

>  	int cmp = (encoded_op >> 24) & 15;

>  	int oparg = (encoded_op << 8) >> 20;

>  	int cmparg = (encoded_op << 20) >> 20;

>  	int oldval = 0, ret, tmp;

> +	u32 __user *uaddr = __uaccess_mask_ptr(_uaddr);

>  

>  	if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))

>  		oparg = 1 << oparg;

> @@ -109,15 +110,17 @@ futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)

>  }

>  

>  static inline int

> -futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,

> +futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *_uaddr,

>  			      u32 oldval, u32 newval)

>  {

>  	int ret = 0;

>  	u32 val, tmp;

> +	u32 __user *uaddr;

>  

> -	if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))

> +	if (!access_ok(VERIFY_WRITE, _uaddr, sizeof(u32)))

>  		return -EFAULT;

>  

> +	uaddr = __uaccess_mask_ptr(_uaddr);

>  	asm volatile("// futex_atomic_cmpxchg_inatomic\n"

>  ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN, CONFIG_ARM64_PAN)

>  "	prfm	pstl1strm, %2\n"


This patch doesn't apply at all as it conflicts with commit
d7c5f8c815466fc00785bbff20f25b39643abe01 which was commit 5f16a046f8e1
("arm64: futex: Fix undefined behaviour with FUTEX_OP_OPARG_SHIFT
usage") upstream.

Any chance you can provide a correct backport of this?

thanks,

greg k-h
Mark Rutland April 18, 2018, 10:56 a.m. | #2
On Tue, Apr 17, 2018 at 02:10:03PM +0200, Greg KH wrote:
> This patch doesn't apply at all as it conflicts with commit

> d7c5f8c815466fc00785bbff20f25b39643abe01 which was commit 5f16a046f8e1

> ("arm64: futex: Fix undefined behaviour with FUTEX_OP_OPARG_SHIFT

> usage") upstream.

> 

> Any chance you can provide a correct backport of this?


The below is rebased atop of the conflicting patch (I based it on
v4.9.94). Luckily it's just a trivial conflict in the function
prototype.

Is this the right way to resend this?

Thanks,
Mark.

---->8----
From 4ff73a6f286a8438529462391eca262b6772e9c1 Mon Sep 17 00:00:00 2001
From: Will Deacon <will.deacon@arm.com>

Date: Mon, 5 Feb 2018 15:34:24 +0000
Subject: [PATCH] arm64: futex: Mask __user pointers prior to dereference

commit 91b2d3442f6a44dce875670d702af22737ad5eff upstream.

The arm64 futex code has some explicit dereferencing of user pointers
where performing atomic operations in response to a futex command. This
patch uses masking to limit any speculative futex operations to within
the user address space.

Signed-off-by: Will Deacon <will.deacon@arm.com>

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>

Signed-off-by: Mark Rutland <mark.rutland@arm.com> [v4.9 backport]

---
 arch/arm64/include/asm/futex.h | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
index 20dcb196b240..4e5f36a804b4 100644
--- a/arch/arm64/include/asm/futex.h
+++ b/arch/arm64/include/asm/futex.h
@@ -51,13 +51,14 @@
 	: "memory")
 
 static inline int
-futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *uaddr)
+futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *_uaddr)
 {
 	int op = (encoded_op >> 28) & 7;
 	int cmp = (encoded_op >> 24) & 15;
 	int oparg = (int)(encoded_op << 8) >> 20;
 	int cmparg = (int)(encoded_op << 20) >> 20;
 	int oldval = 0, ret, tmp;
+	u32 __user *uaddr = __uaccess_mask_ptr(_uaddr);
 
 	if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
 		oparg = 1U << (oparg & 0x1f);
@@ -109,15 +110,17 @@ futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *uaddr)
 }
 
 static inline int
-futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
+futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *_uaddr,
 			      u32 oldval, u32 newval)
 {
 	int ret = 0;
 	u32 val, tmp;
+	u32 __user *uaddr;
 
-	if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
+	if (!access_ok(VERIFY_WRITE, _uaddr, sizeof(u32)))
 		return -EFAULT;
 
+	uaddr = __uaccess_mask_ptr(_uaddr);
 	asm volatile("// futex_atomic_cmpxchg_inatomic\n"
 ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN, CONFIG_ARM64_PAN)
 "	prfm	pstl1strm, %2\n"
-- 
2.11.0
Greg KH April 19, 2018, 7:02 a.m. | #3
On Wed, Apr 18, 2018 at 11:56:36AM +0100, Mark Rutland wrote:
> On Tue, Apr 17, 2018 at 02:10:03PM +0200, Greg KH wrote:

> > This patch doesn't apply at all as it conflicts with commit

> > d7c5f8c815466fc00785bbff20f25b39643abe01 which was commit 5f16a046f8e1

> > ("arm64: futex: Fix undefined behaviour with FUTEX_OP_OPARG_SHIFT

> > usage") upstream.

> > 

> > Any chance you can provide a correct backport of this?

> 

> The below is rebased atop of the conflicting patch (I based it on

> v4.9.94). Luckily it's just a trivial conflict in the function

> prototype.

> 

> Is this the right way to resend this?


That works, now queued up, thanks.

greg k-h

Patch

diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
index f2585cdd32c2..1d123dd01ee0 100644
--- a/arch/arm64/include/asm/futex.h
+++ b/arch/arm64/include/asm/futex.h
@@ -51,13 +51,14 @@ 
 	: "memory")
 
 static inline int
-futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
+futex_atomic_op_inuser (int encoded_op, u32 __user *_uaddr)
 {
 	int op = (encoded_op >> 28) & 7;
 	int cmp = (encoded_op >> 24) & 15;
 	int oparg = (encoded_op << 8) >> 20;
 	int cmparg = (encoded_op << 20) >> 20;
 	int oldval = 0, ret, tmp;
+	u32 __user *uaddr = __uaccess_mask_ptr(_uaddr);
 
 	if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
 		oparg = 1 << oparg;
@@ -109,15 +110,17 @@  futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
 }
 
 static inline int
-futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
+futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *_uaddr,
 			      u32 oldval, u32 newval)
 {
 	int ret = 0;
 	u32 val, tmp;
+	u32 __user *uaddr;
 
-	if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
+	if (!access_ok(VERIFY_WRITE, _uaddr, sizeof(u32)))
 		return -EFAULT;
 
+	uaddr = __uaccess_mask_ptr(_uaddr);
 	asm volatile("// futex_atomic_cmpxchg_inatomic\n"
 ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN, CONFIG_ARM64_PAN)
 "	prfm	pstl1strm, %2\n"