2007-01-18 00:29:07

by Daniel Walker

[permalink] [raw]
Subject: [PATCH] futex null pointer timeout

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 <[email protected]>

---
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);
}
--


2007-01-18 06:45:35

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH] futex null pointer timeout

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 <[email protected]>

Ack.

tglx


2007-01-18 07:39:47

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] futex null pointer timeout


* Daniel Walker <[email protected]> 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

2007-01-18 12:28:33

by Pierre Peiffer

[permalink] [raw]
Subject: Re: [PATCH] futex null pointer timeout

Ingo Molnar a ?crit :
> * Daniel Walker <[email protected]> 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 <[email protected]>

---
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

2007-01-18 12:45:38

by Daniel Walker

[permalink] [raw]
Subject: Re: [PATCH] futex null pointer timeout

On Thu, 2007-01-18 at 13:26 +0100, Pierre Peiffer wrote:
> Ingo Molnar a écrit :
> > * Daniel Walker <[email protected]> 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

2007-01-18 13:08:58

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] futex null pointer timeout


* Pierre Peiffer <[email protected]> wrote:

> Ingo Molnar a ?crit :
> >* Daniel Walker <[email protected]> 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