For all commands except FUTEX_WAIT, timeout is interpreted as an
absolute value. This absolute value is inside the task's time namespace
and has to be converted to the host's time.
Cc: <[email protected]>
Fixes: 5a590f35add9 ("posix-clocks: Wire up clock_gettime() with timens offsets")
Reported-by: Hans van der Laan <[email protected]>
Signed-off-by: Andrei Vagin <[email protected]>
---
kernel/futex.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/futex.c b/kernel/futex.c
index a5876694a60e..9ff2b8c5a506 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -39,6 +39,7 @@
#include <linux/freezer.h>
#include <linux/memblock.h>
#include <linux/fault-inject.h>
+#include <linux/time_namespace.h>
#include <asm/futex.h>
@@ -3797,6 +3798,8 @@ SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
t = timespec64_to_ktime(ts);
if (cmd == FUTEX_WAIT)
t = ktime_add_safe(ktime_get(), t);
+ else if (!(cmd & FUTEX_CLOCK_REALTIME))
+ t = timens_ktime_to_host(CLOCK_MONOTONIC, t);
tp = &t;
}
/*
--
2.26.2
On 10/15/20 8:29 AM, Andrei Vagin wrote:
> For all commands except FUTEX_WAIT, timeout is interpreted as an
> absolute value. This absolute value is inside the task's time namespace
> and has to be converted to the host's time.
>
> Cc: <[email protected]>
> Fixes: 5a590f35add9 ("posix-clocks: Wire up clock_gettime() with timens offsets")
> Reported-by: Hans van der Laan <[email protected]>
> Signed-off-by: Andrei Vagin <[email protected]>[..]
> @@ -3797,6 +3798,8 @@ SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
> t = timespec64_to_ktime(ts);
> if (cmd == FUTEX_WAIT)
> t = ktime_add_safe(ktime_get(), t);
> + else if (!(cmd & FUTEX_CLOCK_REALTIME))
> + t = timens_ktime_to_host(CLOCK_MONOTONIC, t);
Err, it probably should be
: else if (!(op & FUTEX_CLOCK_REALTIME))
And there's also
: SYSCALL_DEFINE6(futex_time32, ...)
which wants to be patched.
Thanks,
Dmitry
On Thu, Oct 15 2020 at 14:26, Dmitry Safonov wrote:
> On 10/15/20 8:29 AM, Andrei Vagin wrote:
>> For all commands except FUTEX_WAIT, timeout is interpreted as an
>> absolute value. This absolute value is inside the task's time namespace
>> and has to be converted to the host's time.
>>
>> Cc: <[email protected]>
>> Fixes: 5a590f35add9 ("posix-clocks: Wire up clock_gettime() with timens offsets")
>> Reported-by: Hans van der Laan <[email protected]>
>> Signed-off-by: Andrei Vagin <[email protected]>[..]
>> @@ -3797,6 +3798,8 @@ SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
>> t = timespec64_to_ktime(ts);
>> if (cmd == FUTEX_WAIT)
>> t = ktime_add_safe(ktime_get(), t);
>> + else if (!(cmd & FUTEX_CLOCK_REALTIME))
>> + t = timens_ktime_to_host(CLOCK_MONOTONIC, t);
>
> Err, it probably should be
> : else if (!(op & FUTEX_CLOCK_REALTIME))
Duh, you are right. I stared at it for a while and did not spot it.
> And there's also
> : SYSCALL_DEFINE6(futex_time32, ...)
> which wants to be patched.
Indeed. I zapped the commits.
Thanks,
tglx
Output on success:
$ ./futex
1..1
ok 1 futex
# Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
Signed-off-by: Andrei Vagin <[email protected]>
---
tools/testing/selftests/timens/Makefile | 2 +-
tools/testing/selftests/timens/futex.c | 107 ++++++++++++++++++++++++
2 files changed, 108 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/timens/futex.c
diff --git a/tools/testing/selftests/timens/Makefile b/tools/testing/selftests/timens/Makefile
index b4fd9a934654..3a5936cc10ab 100644
--- a/tools/testing/selftests/timens/Makefile
+++ b/tools/testing/selftests/timens/Makefile
@@ -1,4 +1,4 @@
-TEST_GEN_PROGS := timens timerfd timer clock_nanosleep procfs exec
+TEST_GEN_PROGS := timens timerfd timer clock_nanosleep procfs exec futex
TEST_GEN_PROGS_EXTENDED := gettime_perf
CFLAGS := -Wall -Werror -pthread
diff --git a/tools/testing/selftests/timens/futex.c b/tools/testing/selftests/timens/futex.c
new file mode 100644
index 000000000000..173760d3fce6
--- /dev/null
+++ b/tools/testing/selftests/timens/futex.c
@@ -0,0 +1,107 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <sched.h>
+
+#include <linux/unistd.h>
+#include <linux/futex.h>
+#include <stdlib.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <string.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <time.h>
+#include <unistd.h>
+
+#include "log.h"
+#include "timens.h"
+
+#define NSEC_PER_SEC 1000000000ULL
+
+static int run_test(void)
+{
+ struct timespec timeout, end;
+ int val = 0;
+
+ clock_gettime(CLOCK_MONOTONIC, &timeout);
+ timeout.tv_nsec += NSEC_PER_SEC / 10; // 100ms
+ if (timeout.tv_nsec > NSEC_PER_SEC) {
+ timeout.tv_sec++;
+ timeout.tv_nsec -= NSEC_PER_SEC;
+ }
+
+ if (syscall(__NR_futex, &val, FUTEX_WAIT_BITSET, 0,
+ &timeout, 0, FUTEX_BITSET_MATCH_ANY) >= 0) {
+ ksft_test_result_fail("futex didn't return ETIMEDOUT");
+ return 1;
+ }
+
+ if (errno != ETIMEDOUT) {
+ ksft_test_result_fail("futex didn't return ETIMEDOUT: %s",
+ strerror(errno));
+ return 1;
+ }
+
+ clock_gettime(CLOCK_MONOTONIC, &end);
+
+ if (end.tv_sec < timeout.tv_sec ||
+ (end.tv_sec == timeout.tv_sec && end.tv_nsec < timeout.tv_sec)) {
+ ksft_test_result_fail("futex slept less than 100ms");
+ return 1;
+ }
+
+
+ ksft_test_result_pass("futex\n");
+
+ return 0;
+}
+
+int main(int argc, char *argv[])
+{
+ int status, len, fd;
+ char buf[4096];
+ pid_t pid;
+ struct timespec mtime_now;
+
+ nscheck();
+
+ check_supported_timers();
+
+ ksft_set_plan(1);
+
+ clock_gettime(CLOCK_MONOTONIC, &mtime_now);
+
+ if (unshare_timens())
+ return 1;
+
+ len = snprintf(buf, sizeof(buf), "%d %d 0",
+ CLOCK_MONOTONIC, 70 * 24 * 3600);
+ fd = open("/proc/self/timens_offsets", O_WRONLY);
+ if (fd < 0)
+ return pr_perror("/proc/self/timens_offsets");
+
+ if (write(fd, buf, len) != len)
+ return pr_perror("/proc/self/timens_offsets");
+
+ close(fd);
+
+ pid = fork();
+ if (pid < 0)
+ return pr_perror("Unable to fork");
+ if (pid == 0) {
+ if (run_test())
+ ksft_exit_fail();
+ ksft_exit_pass();
+ return 0;
+ }
+
+ if (waitpid(pid, &status, 0) != pid)
+ return pr_perror("Unable to wait the child process");
+
+ if (WIFEXITED(status))
+ return WEXITSTATUS(status);
+
+ return 1;
+}
--
2.26.2
The following commit has been merged into the timers/urgent branch of tip:
Commit-ID: 06764291690f8650a9f96dea42cc0dd4138d47d5
Gitweb: https://git.kernel.org/tip/06764291690f8650a9f96dea42cc0dd4138d47d5
Author: Andrei Vagin <[email protected]>
AuthorDate: Thu, 15 Oct 2020 00:29:08 -07:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Thu, 15 Oct 2020 11:24:04 +02:00
futex: Adjust futex absolute timeouts with per-timens offset
For all commands except FUTEX_WAIT, timeout is interpreted as an absolute
value. This absolute value is inside the task's time namespace and has to
be converted to the host's time.
Fixes: 5a590f35add9 ("posix-clocks: Wire up clock_gettime() with timens offsets")
Reported-by: Hans van der Laan <[email protected]>
Signed-off-by: Andrei Vagin <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
kernel/futex.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/futex.c b/kernel/futex.c
index a587669..9ff2b8c 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -39,6 +39,7 @@
#include <linux/freezer.h>
#include <linux/memblock.h>
#include <linux/fault-inject.h>
+#include <linux/time_namespace.h>
#include <asm/futex.h>
@@ -3797,6 +3798,8 @@ SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
t = timespec64_to_ktime(ts);
if (cmd == FUTEX_WAIT)
t = ktime_add_safe(ktime_get(), t);
+ else if (!(cmd & FUTEX_CLOCK_REALTIME))
+ t = timens_ktime_to_host(CLOCK_MONOTONIC, t);
tp = &t;
}
/*
The following commit has been merged into the timers/urgent branch of tip:
Commit-ID: 4cdf42596216e08051c0ccc4c896dcb8b2d22f10
Gitweb: https://git.kernel.org/tip/4cdf42596216e08051c0ccc4c896dcb8b2d22f10
Author: Andrei Vagin <[email protected]>
AuthorDate: Thu, 15 Oct 2020 00:29:09 -07:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Thu, 15 Oct 2020 11:24:04 +02:00
selftests/timens: Add a test for futex()
Output on success:
$ ./futex
1..1
ok 1 futex
# Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
Signed-off-by: Andrei Vagin <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
tools/testing/selftests/timens/Makefile | 2 +-
tools/testing/selftests/timens/futex.c | 107 +++++++++++++++++++++++-
2 files changed, 108 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/timens/futex.c
diff --git a/tools/testing/selftests/timens/Makefile b/tools/testing/selftests/timens/Makefile
index b4fd9a9..3a5936c 100644
--- a/tools/testing/selftests/timens/Makefile
+++ b/tools/testing/selftests/timens/Makefile
@@ -1,4 +1,4 @@
-TEST_GEN_PROGS := timens timerfd timer clock_nanosleep procfs exec
+TEST_GEN_PROGS := timens timerfd timer clock_nanosleep procfs exec futex
TEST_GEN_PROGS_EXTENDED := gettime_perf
CFLAGS := -Wall -Werror -pthread
diff --git a/tools/testing/selftests/timens/futex.c b/tools/testing/selftests/timens/futex.c
new file mode 100644
index 0000000..173760d
--- /dev/null
+++ b/tools/testing/selftests/timens/futex.c
@@ -0,0 +1,107 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <sched.h>
+
+#include <linux/unistd.h>
+#include <linux/futex.h>
+#include <stdlib.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <string.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <time.h>
+#include <unistd.h>
+
+#include "log.h"
+#include "timens.h"
+
+#define NSEC_PER_SEC 1000000000ULL
+
+static int run_test(void)
+{
+ struct timespec timeout, end;
+ int val = 0;
+
+ clock_gettime(CLOCK_MONOTONIC, &timeout);
+ timeout.tv_nsec += NSEC_PER_SEC / 10; // 100ms
+ if (timeout.tv_nsec > NSEC_PER_SEC) {
+ timeout.tv_sec++;
+ timeout.tv_nsec -= NSEC_PER_SEC;
+ }
+
+ if (syscall(__NR_futex, &val, FUTEX_WAIT_BITSET, 0,
+ &timeout, 0, FUTEX_BITSET_MATCH_ANY) >= 0) {
+ ksft_test_result_fail("futex didn't return ETIMEDOUT");
+ return 1;
+ }
+
+ if (errno != ETIMEDOUT) {
+ ksft_test_result_fail("futex didn't return ETIMEDOUT: %s",
+ strerror(errno));
+ return 1;
+ }
+
+ clock_gettime(CLOCK_MONOTONIC, &end);
+
+ if (end.tv_sec < timeout.tv_sec ||
+ (end.tv_sec == timeout.tv_sec && end.tv_nsec < timeout.tv_sec)) {
+ ksft_test_result_fail("futex slept less than 100ms");
+ return 1;
+ }
+
+
+ ksft_test_result_pass("futex\n");
+
+ return 0;
+}
+
+int main(int argc, char *argv[])
+{
+ int status, len, fd;
+ char buf[4096];
+ pid_t pid;
+ struct timespec mtime_now;
+
+ nscheck();
+
+ check_supported_timers();
+
+ ksft_set_plan(1);
+
+ clock_gettime(CLOCK_MONOTONIC, &mtime_now);
+
+ if (unshare_timens())
+ return 1;
+
+ len = snprintf(buf, sizeof(buf), "%d %d 0",
+ CLOCK_MONOTONIC, 70 * 24 * 3600);
+ fd = open("/proc/self/timens_offsets", O_WRONLY);
+ if (fd < 0)
+ return pr_perror("/proc/self/timens_offsets");
+
+ if (write(fd, buf, len) != len)
+ return pr_perror("/proc/self/timens_offsets");
+
+ close(fd);
+
+ pid = fork();
+ if (pid < 0)
+ return pr_perror("Unable to fork");
+ if (pid == 0) {
+ if (run_test())
+ ksft_exit_fail();
+ ksft_exit_pass();
+ return 0;
+ }
+
+ if (waitpid(pid, &status, 0) != pid)
+ return pr_perror("Unable to wait the child process");
+
+ if (WIFEXITED(status))
+ return WEXITSTATUS(status);
+
+ return 1;
+}
On Thu, Oct 15, 2020 at 04:13:42PM +0200, Thomas Gleixner wrote:
> On Thu, Oct 15 2020 at 14:26, Dmitry Safonov wrote:
>
> > On 10/15/20 8:29 AM, Andrei Vagin wrote:
> >> For all commands except FUTEX_WAIT, timeout is interpreted as an
> >> absolute value. This absolute value is inside the task's time namespace
> >> and has to be converted to the host's time.
> >>
> >> Cc: <[email protected]>
> >> Fixes: 5a590f35add9 ("posix-clocks: Wire up clock_gettime() with timens offsets")
> >> Reported-by: Hans van der Laan <[email protected]>
> >> Signed-off-by: Andrei Vagin <[email protected]>[..]
> >> @@ -3797,6 +3798,8 @@ SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
> >> t = timespec64_to_ktime(ts);
> >> if (cmd == FUTEX_WAIT)
> >> t = ktime_add_safe(ktime_get(), t);
> >> + else if (!(cmd & FUTEX_CLOCK_REALTIME))
> >> + t = timens_ktime_to_host(CLOCK_MONOTONIC, t);
> >
> > Err, it probably should be
> > : else if (!(op & FUTEX_CLOCK_REALTIME))
Dmitry, thank you for catching this.
>
> Duh, you are right. I stared at it for a while and did not spot it.
>
> > And there's also
> > : SYSCALL_DEFINE6(futex_time32, ...)
> > which wants to be patched.
>
> Indeed. I zapped the commits.
I sent a new version. This time, I extended the test to check
FUTEX_CLOCK_REALTIME and I compiled and run the compat version.
Everything works as it should be.
Thanks,
Andrei