diff mbox

[Lucid] UBUNTU: SAUCE: (no-up) Fix regression introduced by patch, for CVE-2014-3153

Message ID 5391F406.2010104@canonical.com
State New
Headers show

Commit Message

John Johansen June 6, 2014, 5:01 p.m. UTC
From 0695328167baa4241b1f17b4d0d0a38c7b06981e Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Fri, 6 Jun 2014 17:41:09 +0100
Subject: [PATCH] UBUNTU: SAUCE: (no-up) Fix regression introduced by patch
 for CVE-2014-3153

Phil Turnbull reported a problem with the Lucid (2.6.32) backport of
  futex: Always cleanup owner tid in unlock_pi
  commit: 8e4e453d548e3c24e9070eda23c52f210951b921

In patches-2.6.32.tgz:patches/0003-futex-Always-cleanup-owner-tid-in-unlock_pi.patch
there is this change (ignoring whitespace changes):

        curval = cmpxchg_futex_value_locked(uaddr, uval, newval);
-
-               if (curval == -EFAULT)
+       if (curval)
                ret = -EFAULT;

which seems to change the behaviour of the function.

The purpose of the return value of cmpxchg_futex_value_locked changed in

37a9d912b24f96a0591 "futex: Sanitize cmpxchg_futex_value_locked API"

which is not included in 2.6.32. This patch changes the return value to a
status code, but in 2.6.32 the return value is the value of the futex or
-EFAULT. With this backported patch, any futex with a non-zero value will
return -EFAULT.

BugLink: http://bugs.launchpad.net/bugs/1327300

Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 kernel/futex.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Brad Figg June 6, 2014, 5:07 p.m. UTC | #1
On 06/06/2014 10:01 AM, John Johansen wrote:
> From 0695328167baa4241b1f17b4d0d0a38c7b06981e Mon Sep 17 00:00:00 2001
> From: John Johansen <john.johansen@canonical.com>
> Date: Fri, 6 Jun 2014 17:41:09 +0100
> Subject: [PATCH] UBUNTU: SAUCE: (no-up) Fix regression introduced by patch
>  for CVE-2014-3153
> 
> Phil Turnbull reported a problem with the Lucid (2.6.32) backport of
>   futex: Always cleanup owner tid in unlock_pi
>   commit: 8e4e453d548e3c24e9070eda23c52f210951b921
> 
> In patches-2.6.32.tgz:patches/0003-futex-Always-cleanup-owner-tid-in-unlock_pi.patch
> there is this change (ignoring whitespace changes):
> 
>         curval = cmpxchg_futex_value_locked(uaddr, uval, newval);
> -
> -               if (curval == -EFAULT)
> +       if (curval)
>                 ret = -EFAULT;
> 
> which seems to change the behaviour of the function.
> 
> The purpose of the return value of cmpxchg_futex_value_locked changed in
> 
> 37a9d912b24f96a0591 "futex: Sanitize cmpxchg_futex_value_locked API"
> 
> which is not included in 2.6.32. This patch changes the return value to a
> status code, but in 2.6.32 the return value is the value of the futex or
> -EFAULT. With this backported patch, any futex with a non-zero value will
> return -EFAULT.
> 
> BugLink: http://bugs.launchpad.net/bugs/1327300
> 
> Signed-off-by: John Johansen <john.johansen@canonical.com>
> ---
>  kernel/futex.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/futex.c b/kernel/futex.c
> index ae85d66..cd50a44 100644
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -932,7 +932,7 @@ static int wake_futex_pi(u32 __user *uaddr, u32 uval, struct futex_q *this)
>  	newval = FUTEX_WAITERS | task_pid_vnr(new_owner);
>  
>  	curval = cmpxchg_futex_value_locked(uaddr, uval, newval);
> -	if (curval)
> +	if (curval == -EFAULT)
>  		ret = -EFAULT;
>  	else if (curval != uval)
>  		ret = -EINVAL;
>
Tim Gardner June 6, 2014, 5:16 p.m. UTC | #2

diff mbox

Patch

diff --git a/kernel/futex.c b/kernel/futex.c
index ae85d66..cd50a44 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -932,7 +932,7 @@  static int wake_futex_pi(u32 __user *uaddr, u32 uval, struct futex_q *this)
 	newval = FUTEX_WAITERS | task_pid_vnr(new_owner);
 
 	curval = cmpxchg_futex_value_locked(uaddr, uval, newval);
-	if (curval)
+	if (curval == -EFAULT)
 		ret = -EFAULT;
 	else if (curval != uval)
 		ret = -EINVAL;