[PATCH] futex null pointer timeout

[PATCH] futex null pointer timeout

[Daniel Walker]

This fix is mostly from Thomas .. 

The problem was that a futex can be called with a zero timeout (0 seconds,
0 nanoseconds) and it's a valid expired timeout. However, the current futex
in -rt assumes a zero timeout is an infinite timeout. 

Kevin Hilman found this using LTP's nptl01 test case which would
soft hang occasionally. 

The patch reworks do_futex, and futex_wait* so a NULL pointer in the timeout
position is infinite, and anything else is evaluated as a real timeout.

Signed-Off-By: Daniel Walker <dwalker@mvista.com>

---
 kernel/futex.c        |   14 ++++++++------
 kernel/futex_compat.c |    5 +++--
 2 files changed, 11 insertions(+), 8 deletions(-)

Index: linux-2.6.19/kernel/futex.c
===================================================================
--- linux-2.6.19.orig/kernel/futex.c
+++ linux-2.6.19/kernel/futex.c
@@ -1466,7 +1466,7 @@ static int futex_wait(u32 __user *uaddr,
 
 		current->flags &= ~PF_NOSCHED;
 
-		if (time->tv_sec == 0 && time->tv_nsec == 0)
+		if (!time)
 			schedule();
 		else {
 			to = &t;
@@ -3070,7 +3070,7 @@ static int futex_wait64(u64 __user *uadd
 
 		current->flags &= ~PF_NOSCHED;
 
-		if (time->tv_sec == 0 && time->tv_nsec == 0)
+		if (!time)
 			schedule();
 		else {
 			to = &t;
@@ -3560,7 +3560,7 @@ asmlinkage long
 sys_futex64(u64 __user *uaddr, int op, u64 val,
 	    struct timespec __user *utime, u64 __user *uaddr2, u64 val3)
 {
-	struct timespec t = {.tv_sec=0, .tv_nsec=0};
+	struct timespec t, *tp = NULL;
 	u64 val2 = 0;
 
 	if (utime && (op == FUTEX_WAIT || op == FUTEX_LOCK_PI)) {
@@ -3568,6 +3568,7 @@ sys_futex64(u64 __user *uaddr, int op, u
 			return -EFAULT;
 		if (!timespec_valid(&t))
 			return -EINVAL;
+		tp = &t;
 	}
 	/*
 	 * requeue parameter in 'utime' if op == FUTEX_REQUEUE.
@@ -3576,7 +3577,7 @@ sys_futex64(u64 __user *uaddr, int op, u
 	    || op == FUTEX_CMP_REQUEUE_PI)
 		val2 = (unsigned long) utime;
 
-	return do_futex64(uaddr, op, val, &t, uaddr2, val2, val3);
+	return do_futex64(uaddr, op, val, tp, uaddr2, val2, val3);
 }
 
 #endif
@@ -3585,7 +3586,7 @@ asmlinkage long sys_futex(u32 __user *ua
 			  struct timespec __user *utime, u32 __user *uaddr2,
 			  u32 val3)
 {
-	struct timespec t = {.tv_sec=0, .tv_nsec=0};
+	struct timespec t, *tp = NULL;
 	u32 val2 = 0;
 
 	if (utime && (op == FUTEX_WAIT || op == FUTEX_LOCK_PI)) {
@@ -3593,6 +3594,7 @@ asmlinkage long sys_futex(u32 __user *ua
 			return -EFAULT;
 		if (!timespec_valid(&t))
 			return -EINVAL;
+		tp = &t;
 	}
 	/*
 	 * requeue parameter in 'utime' if op == FUTEX_REQUEUE.
@@ -3601,7 +3603,7 @@ asmlinkage long sys_futex(u32 __user *ua
 	    || op == FUTEX_CMP_REQUEUE_PI)
 		val2 = (u32) (unsigned long) utime;
 
-	return do_futex(uaddr, op, val, &t, uaddr2, val2, val3);
+	return do_futex(uaddr, op, val, tp, uaddr2, val2, val3);
 }
 
 static int futexfs_get_sb(struct file_system_type *fs_type,
Index: linux-2.6.19/kernel/futex_compat.c
===================================================================
--- linux-2.6.19.orig/kernel/futex_compat.c
+++ linux-2.6.19/kernel/futex_compat.c
@@ -141,7 +141,7 @@ asmlinkage long compat_sys_futex(u32 __u
 		struct compat_timespec __user *utime, u32 __user *uaddr2,
 		u32 val3)
 {
-	struct timespec t = {.tv_sec = 0, .tv_nsec = 0};
+	struct timespec t, *tp = NULL;
 	int val2 = 0;
 
 	if (utime && (op == FUTEX_WAIT || op == FUTEX_LOCK_PI)) {
@@ -149,10 +149,11 @@ asmlinkage long compat_sys_futex(u32 __u
 			return -EFAULT;
 		if (!timespec_valid(&t))
 			return -EINVAL;
+		tp = &t;
 	}
 	if (op == FUTEX_REQUEUE || op == FUTEX_CMP_REQUEUE
 	    || op == FUTEX_CMP_REQUEUE_PI)
 		val2 = (int) (unsigned long) utime;
 
-	return do_futex(uaddr, op, val, &t, uaddr2, val2, val3);
+	return do_futex(uaddr, op, val, tp, uaddr2, val2, val3);
 }
-- 

Re: [PATCH] futex null pointer timeout

[Thomas Gleixner]

On Wed, 2007-01-17 at 16:25 -0800, Daniel Walker wrote:
> The patch reworks do_futex, and futex_wait* so a NULL pointer in the timeout
> position is infinite, and anything else is evaluated as a real timeout.
> 
> Signed-Off-By: Daniel Walker <dwalker@mvista.com>

Ack.

	tglx

Re: [PATCH] futex null pointer timeout

[Ingo Molnar]

* Daniel Walker <dwalker@mvista.com> wrote:

> This fix is mostly from Thomas ..
> 
> The problem was that a futex can be called with a zero timeout (0 
> seconds, 0 nanoseconds) and it's a valid expired timeout. However, the 
> current futex in -rt assumes a zero timeout is an infinite timeout.
> 
> Kevin Hilman found this using LTP's nptl01 test case which would soft 
> hang occasionally.
> 
> The patch reworks do_futex, and futex_wait* so a NULL pointer in the 
> timeout position is infinite, and anything else is evaluated as a real 
> timeout.

thanks, applied.

	Ingo

Re: [PATCH] futex null pointer timeout

[Pierre Peiffer]

Ingo Molnar a écrit :
> * Daniel Walker <dwalker@mvista.com> wrote:
> 
[...]
>> The patch reworks do_futex, and futex_wait* so a NULL pointer in the 
>> timeout position is infinite, and anything else is evaluated as a real 
>> timeout.
> 
> thanks, applied.
> 

On top of this patch, you will need the following patch: futex_lock_pi is also 
involved.

---
  futex.c |    4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
---

Signed-off-by: Pierre Peiffer <pierre.peiffer@bull.net>

---
Index: linux-2.6/kernel/futex.c
===================================================================
--- linux-2.6.orig/kernel/futex.c	2007-01-18 13:16:32.000000000 +0100
+++ linux-2.6/kernel/futex.c	2007-01-18 13:19:32.000000000 +0100
@@ -1644,7 +1644,7 @@ static int futex_lock_pi(u32 __user *uad
  	if (refill_pi_state_cache())
  		return -ENOMEM;

-	if (time->tv_sec || time->tv_nsec) {
+	if (time) {
  		to = &timeout;
  		hrtimer_init(&to->timer, CLOCK_REALTIME, HRTIMER_MODE_ABS);
  		hrtimer_init_sleeper(to, current);
@@ -3197,7 +3197,7 @@ static int futex_lock_pi64(u64 __user *u
  	if (refill_pi_state_cache())
  		return -ENOMEM;

-	if (time->tv_sec || time->tv_nsec) {
+	if (time) {
  		to = &timeout;
  		hrtimer_init(&to->timer, CLOCK_REALTIME, HRTIMER_MODE_ABS);
  		hrtimer_init_sleeper(to, current);

-- 
Pierre

Re: [PATCH] futex null pointer timeout

[Daniel Walker]

On Thu, 2007-01-18 at 13:26 +0100, Pierre Peiffer wrote:
> Ingo Molnar a écrit :
> > * Daniel Walker <dwalker@mvista.com> wrote:
> > 
> [...]
> >> The patch reworks do_futex, and futex_wait* so a NULL pointer in the 
> >> timeout position is infinite, and anything else is evaluated as a real 
> >> timeout.
> > 
> > thanks, applied.
> > 
> 
> On top of this patch, you will need the following patch: futex_lock_pi is also 
> involved.
> 

True.

Daniel

Re: [PATCH] futex null pointer timeout

[Ingo Molnar]

* Pierre Peiffer <pierre.peiffer@bull.net> wrote:

> Ingo Molnar a écrit :
> >* Daniel Walker <dwalker@mvista.com> wrote:
> >
> [...]
> >>The patch reworks do_futex, and futex_wait* so a NULL pointer in the 
> >>timeout position is infinite, and anything else is evaluated as a real 
> >>timeout.
> >
> >thanks, applied.
> >
> 
> On top of this patch, you will need the following patch: futex_lock_pi 
> is also involved.

thanks, applied. (FYI, your mailer added an extra space to every context 
line in the patch and thus corrupted it, i fixed it up by hand.)

	Ingo