[LTP] [PATCH v3] openposix: timer_*/speculative: Don't pass a stack pointer as timerid

[LTP] [PATCH v3] openposix: timer_*/speculative: Don't pass a stack pointer as timerid

[Avinesh Kumar via ltp]

From: Avinesh Kumar <avinesh.kumar@suse.com>

timer_delete/speculative/5-1, timer_getoverrun/speculative/6-1,
timer_gettime/speculative/6-1, and timer_settime/speculative/12-1 all
construct an invalid timerid from the address of a local variable:

  int tval = BOGUSTIMERID;
  tid = (timer_t) & tval;

glibc's timer_t is overloadedp[0]: for !SIGEV_THREAD timers it is
the kernel-assined timer ID, while for SIGEV_THREAD timers it is a
tagged pointer into glibc's own internal state. Before issuing any
syscall, timerid_to_kernel_timer() picks between the two using only
the sign bit of the value:

  if (timer_is_sigev_thread (timerid))
    return timerid_to_timer (timerid)->ktimerid;
  else
    return (kernel_timer_t) ((uintptr_t) timerid);

On i586, an ordinary stack address can have its sign bit set,
so glibc mistakes &tval for a tagged pointer, derives an unrelated,
fabricated address from it via a bit shift, and dereferences it -
crashing inside libc before the kernel is ever reached:

  timer_delete_sp[22499]: segfault at 7f4982f0 ip b7e07824 sp bfa4c140

A small integer value like the ones we use for bogus IDs in these tests
will never have its sign bit set on any architecture, so it will always
take the safe "just an int" path and will be correctly rejected by the
kernel's own timer lookup with a genuine EINVAL.

[0] https://codebrowser.dev/glibc/glibc/sysdeps/unix/sysv/linux/kernel-posix-timers.h.html

Signed-off-by: Avinesh Kumar <avinesh.kumar@suse.com>
---
 .../conformance/interfaces/timer_delete/speculative/5-1.c    | 4 +---
 .../interfaces/timer_getoverrun/speculative/6-1.c            | 4 +---
 .../conformance/interfaces/timer_gettime/speculative/6-1.c   | 5 ++---
 .../conformance/interfaces/timer_settime/speculative/12-1.c  | 4 +---
 4 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/testcases/open_posix_testsuite/conformance/interfaces/timer_delete/speculative/5-1.c b/testcases/open_posix_testsuite/conformance/interfaces/timer_delete/speculative/5-1.c
index 912cf5800e6f..dce9277b5d6a 100644
--- a/testcases/open_posix_testsuite/conformance/interfaces/timer_delete/speculative/5-1.c
+++ b/testcases/open_posix_testsuite/conformance/interfaces/timer_delete/speculative/5-1.c
@@ -19,9 +19,7 @@
 
 int test_main(int argc PTS_ATTRIBUTE_UNUSED, char **argv PTS_ATTRIBUTE_UNUSED)
 {
-	timer_t tid;
-	int tval = BOGUSTIMERID;
-	tid = (timer_t) & tval;
+	timer_t tid = (timer_t)BOGUSTIMERID;
 
 	if (timer_delete(tid) == -1) {
 		if (errno == EINVAL) {
diff --git a/testcases/open_posix_testsuite/conformance/interfaces/timer_getoverrun/speculative/6-1.c b/testcases/open_posix_testsuite/conformance/interfaces/timer_getoverrun/speculative/6-1.c
index 6e18560e5084..3a8f1448d4d4 100644
--- a/testcases/open_posix_testsuite/conformance/interfaces/timer_getoverrun/speculative/6-1.c
+++ b/testcases/open_posix_testsuite/conformance/interfaces/timer_getoverrun/speculative/6-1.c
@@ -19,9 +19,7 @@
 
 int test_main(int argc PTS_ATTRIBUTE_UNUSED, char **argv PTS_ATTRIBUTE_UNUSED)
 {
-	timer_t tid;
-	int tval = BOGUSTID;
-	tid = (timer_t) & tval;
+	timer_t tid = (timer_t)BOGUSTID;
 
 	if (timer_getoverrun(tid) == -1) {
 		if (EINVAL == errno) {
diff --git a/testcases/open_posix_testsuite/conformance/interfaces/timer_gettime/speculative/6-1.c b/testcases/open_posix_testsuite/conformance/interfaces/timer_gettime/speculative/6-1.c
index d09c2f70901d..586b0ed3a2b1 100644
--- a/testcases/open_posix_testsuite/conformance/interfaces/timer_gettime/speculative/6-1.c
+++ b/testcases/open_posix_testsuite/conformance/interfaces/timer_gettime/speculative/6-1.c
@@ -19,10 +19,9 @@
 
 int test_main(int argc PTS_ATTRIBUTE_UNUSED, char **argv PTS_ATTRIBUTE_UNUSED)
 {
-	timer_t tid;
 	struct itimerspec its;
-	int tval = BOGUSTID;
-	tid = (timer_t) & tval;
+	timer_t tid = (timer_t)BOGUSTID;
+
 	if (timer_gettime(tid, &its) == -1) {
 		if (EINVAL == errno) {
 			printf("fcn returned -1 and errno==EINVAL\n");
diff --git a/testcases/open_posix_testsuite/conformance/interfaces/timer_settime/speculative/12-1.c b/testcases/open_posix_testsuite/conformance/interfaces/timer_settime/speculative/12-1.c
index 5d4e1dda30ba..ac7f7bf24c39 100644
--- a/testcases/open_posix_testsuite/conformance/interfaces/timer_settime/speculative/12-1.c
+++ b/testcases/open_posix_testsuite/conformance/interfaces/timer_settime/speculative/12-1.c
@@ -18,10 +18,8 @@
 
 int test_main(int argc PTS_ATTRIBUTE_UNUSED, char **argv PTS_ATTRIBUTE_UNUSED)
 {
-	timer_t tid;
 	struct itimerspec its;
-	int tval = BOGUSTID;
-	tid = (timer_t) & tval;
+	timer_t tid = (timer_t)BOGUSTID;
 	its.it_interval.tv_sec = 0;
 	its.it_interval.tv_nsec = 0;
 	its.it_value.tv_sec = 0;
-- 
2.55.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] openposix: timer_*/speculative: Don't pass a stack pointer as timerid

[Cyril Hrubis]

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] openposix: timer_*/speculative: Don't pass a stack pointer as timerid

[Andrea Cervesato via ltp]

Reviewed-by: Andrea Cervesato <andrea.cervesato@suse.com>

--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] openposix: timer_*/speculative: Don't pass a stack pointer as timerid

[Andrea Cervesato via ltp]

Merged, Thanks!

--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp