[PATCH 0/7] y2038: cond_wait_prologue64 and related fixes

[PATCH 0/7] y2038: cond_wait_prologue64 and related fixes

[Florian Bezdeka]

Hi all,

this is the last missing POSIX related y2038 affected syscall in
Xenomai. With this applied we have two Xenomai specific syscalls
missing:

  - sc_cobalt_thread_setschedparam_ex
  - sc_cobalt_thread_getschedparam_ex

While adding tests for the introduced cond_wait_prologue64 I hit a
kernel OOPS due to insuficient validation of user provided pointers.
That has been addressed as well.

This series has been tested against all Xenomai 3.3 supported
architectures / kernels. ipipe based kernels were out of scope (which
should not make any difference).

Best regards,
Florian

Florian Bezdeka (7):
  y2038: cobalt/posix/cond: Adding cond_wait_prologue64
  cobalt: posix/cond: Add missing __user annotation to user provided ptr
  y2038: lib/cobalt: Dispatch cond_wait_prologue
  cobalt: posix/cond: Add missing input validations
  y2038: testsuite/smokey/y2038: Adding tests for cond_wait_prologue64
  y2038: testsuite/smokey/y2038: Add a missing error handling path
  cobalt: Protect __xn_get_user() by access_ok()

 include/cobalt/uapi/syscall.h          |   1 +
 kernel/cobalt/posix/cond.c             |  54 ++++++++++---
 kernel/cobalt/posix/cond.h             |  17 ++++-
 kernel/cobalt/posix/internal.h         |   4 +
 kernel/cobalt/posix/nsem.c             |   3 +-
 kernel/cobalt/posix/syscall32.c        |  15 +++-
 kernel/cobalt/posix/syscall32.h        |   9 ++-
 kernel/cobalt/trace/cobalt-posix.h     |   3 +-
 lib/cobalt/cond.c                      |  21 +++++-
 testsuite/smokey/y2038/syscall-tests.c | 100 ++++++++++++++++++++++++-
 10 files changed, 206 insertions(+), 21 deletions(-)

-- 
2.30.2

[PATCH 1/7] y2038: cobalt/posix/cond: Adding cond_wait_prologue64

[Florian Bezdeka]

Add a syscall specific for cond_wait_prologue64 with 64bit time_t.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 include/cobalt/uapi/syscall.h      |  1 +
 kernel/cobalt/posix/cond.c         | 26 ++++++++++++++++++++++++++
 kernel/cobalt/posix/cond.h         | 13 +++++++++++++
 kernel/cobalt/posix/syscall32.c    | 10 ++++++++++
 kernel/cobalt/posix/syscall32.h    |  7 +++++++
 kernel/cobalt/trace/cobalt-posix.h |  3 ++-
 6 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/include/cobalt/uapi/syscall.h b/include/cobalt/uapi/syscall.h
index 1523ddd2d..9646a0d97 100644
--- a/include/cobalt/uapi/syscall.h
+++ b/include/cobalt/uapi/syscall.h
@@ -135,6 +135,7 @@
 #define sc_cobalt_monitor_wait64		112
 #define sc_cobalt_event_wait64			113
 #define sc_cobalt_recvmmsg64			114
+#define sc_cobalt_cond_wait_prologue64		115
 
 #define __NR_COBALT_SYSCALLS			128 /* Power of 2 */
 
diff --git a/kernel/cobalt/posix/cond.c b/kernel/cobalt/posix/cond.c
index bb18fe316..28400505f 100644
--- a/kernel/cobalt/posix/cond.c
+++ b/kernel/cobalt/posix/cond.c
@@ -21,6 +21,7 @@
 #include "mutex.h"
 #include "cond.h"
 #include "clock.h"
+#include <cobalt/kernel/time.h>
 #include <trace/events/cobalt-posix.h>
 
 static inline int
@@ -276,6 +277,12 @@ static inline int cond_fetch_timeout(struct timespec64 *ts,
 	return u_ts == NULL ? -EFAULT :	cobalt_get_u_timespec(ts, u_ts);
 }
 
+static inline int cond_fetch_timeout64(struct timespec64 *ts,
+				       const void __user *u_ts)
+{
+	return u_ts == NULL ? -EFAULT : cobalt_get_timespec64(ts, u_ts);
+}
+
 int __cobalt_cond_wait_prologue(struct cobalt_cond_shadow __user *u_cnd,
 				struct cobalt_mutex_shadow __user *u_mx,
 				int *u_err,
@@ -342,6 +349,15 @@ int __cobalt_cond_wait_prologue(struct cobalt_cond_shadow __user *u_cnd,
 	return err == 0 ? perr : err;
 }
 
+int __cobalt_cond_wait_prologue64(struct cobalt_cond_shadow __user *u_cnd,
+				  struct cobalt_mutex_shadow __user *u_mx,
+				  int *u_err, unsigned int timed,
+				  void __user *u_ts)
+{
+	return __cobalt_cond_wait_prologue(u_cnd, u_mx, u_err, u_ts,
+					   timed ? cond_fetch_timeout64 : NULL);
+}
+
 /* pthread_cond_wait_prologue(cond, mutex, count_ptr, timed, timeout) */
 COBALT_SYSCALL(cond_wait_prologue, nonrestartable,
 	       (struct cobalt_cond_shadow __user *u_cnd,
@@ -354,6 +370,16 @@ COBALT_SYSCALL(cond_wait_prologue, nonrestartable,
 					   timed ? cond_fetch_timeout : NULL);
 }
 
+COBALT_SYSCALL(cond_wait_prologue64, nonrestartable,
+	       (struct cobalt_cond_shadow __user *u_cnd,
+		struct cobalt_mutex_shadow __user *u_mx,
+		int *u_err,
+		unsigned int timed,
+		struct __kernel_timespec __user *u_ts))
+{
+	return __cobalt_cond_wait_prologue64(u_cnd, u_mx, u_err, timed, u_ts);
+}
+
 COBALT_SYSCALL(cond_wait_epilogue, primary,
 	       (struct cobalt_cond_shadow __user *u_cnd,
 		struct cobalt_mutex_shadow __user *u_mx))
diff --git a/kernel/cobalt/posix/cond.h b/kernel/cobalt/posix/cond.h
index 7bec2a649..b087b189e 100644
--- a/kernel/cobalt/posix/cond.h
+++ b/kernel/cobalt/posix/cond.h
@@ -45,6 +45,12 @@ int __cobalt_cond_wait_prologue(struct cobalt_cond_shadow __user *u_cnd,
 				void __user *u_ts,
 				int (*fetch_timeout)(struct timespec64 *ts,
 						     const void __user *u_ts));
+
+int __cobalt_cond_wait_prologue64(struct cobalt_cond_shadow __user *u_cnd,
+				  struct cobalt_mutex_shadow __user *u_mx,
+				  int *u_err, unsigned int timed,
+				  void __user *u_ts);
+
 COBALT_SYSCALL_DECL(cond_init,
 		    (struct cobalt_cond_shadow __user *u_cnd,
 		     const struct cobalt_condattr __user *u_attr));
@@ -59,6 +65,13 @@ COBALT_SYSCALL_DECL(cond_wait_prologue,
 		     unsigned int timed,
 		     struct __user_old_timespec __user *u_ts));
 
+COBALT_SYSCALL_DECL(cond_wait_prologue64,
+		    (struct cobalt_cond_shadow __user *u_cnd,
+		     struct cobalt_mutex_shadow __user *u_mx,
+		     int *u_err,
+		     unsigned int timed,
+		     struct __kernel_timespec __user *u_ts));
+
 COBALT_SYSCALL_DECL(cond_wait_epilogue,
 		    (struct cobalt_cond_shadow __user *u_cnd,
 		     struct cobalt_mutex_shadow __user *u_mx));
diff --git a/kernel/cobalt/posix/syscall32.c b/kernel/cobalt/posix/syscall32.c
index 266789611..fbd2b7f79 100644
--- a/kernel/cobalt/posix/syscall32.c
+++ b/kernel/cobalt/posix/syscall32.c
@@ -286,6 +286,16 @@ COBALT_SYSCALL32emu(cond_wait_prologue, nonrestartable,
 					   timed ? sys32_fetch_timeout : NULL);
 }
 
+COBALT_SYSCALL32emu(cond_wait_prologue64, nonrestartable,
+		    (struct cobalt_cond_shadow __user *u_cnd,
+		     struct cobalt_mutex_shadow __user *u_mx,
+		     int *u_err,
+		     unsigned int timed,
+		     struct __kernel_timespec __user *u_ts))
+{
+	return __cobalt_cond_wait_prologue64(u_cnd, u_mx, u_err, timed, u_ts);
+}
+
 COBALT_SYSCALL32emu(mq_open, lostage,
 		    (const char __user *u_name, int oflags,
 		     mode_t mode, struct compat_mq_attr __user *u_attr))
diff --git a/kernel/cobalt/posix/syscall32.h b/kernel/cobalt/posix/syscall32.h
index 72e32f4f8..cdaa903ea 100644
--- a/kernel/cobalt/posix/syscall32.h
+++ b/kernel/cobalt/posix/syscall32.h
@@ -109,6 +109,13 @@ COBALT_SYSCALL32emu_DECL(cond_wait_prologue,
 			  unsigned int timed,
 			  struct old_timespec32 __user *u_ts));
 
+COBALT_SYSCALL32emu_DECL(cond_wait_prologue64,
+			 (struct cobalt_cond_shadow __user *u_cnd,
+			  struct cobalt_mutex_shadow __user *u_mx,
+			  int *u_err,
+			  unsigned int timed,
+			  struct __kernel_timespec __user *u_ts));
+
 COBALT_SYSCALL32emu_DECL(mq_open,
 			 (const char __user *u_name, int oflags,
 			  mode_t mode, struct compat_mq_attr __user *u_attr));
diff --git a/kernel/cobalt/trace/cobalt-posix.h b/kernel/cobalt/trace/cobalt-posix.h
index 2bc004dab..c7eef7fba 100644
--- a/kernel/cobalt/trace/cobalt-posix.h
+++ b/kernel/cobalt/trace/cobalt-posix.h
@@ -167,7 +167,8 @@
 		__cobalt_symbolic_syscall(sigtimedwait64),		\
 		__cobalt_symbolic_syscall(monitor_wait64),		\
 		__cobalt_symbolic_syscall(event_wait64),		\
-		__cobalt_symbolic_syscall(recvmmsg64))
+		__cobalt_symbolic_syscall(recvmmsg64),			\
+		__cobalt_symbolic_syscall(cond_wait_prologue64))
 
 DECLARE_EVENT_CLASS(cobalt_syscall_entry,
 	TP_PROTO(unsigned int nr),
-- 
2.30.2

[PATCH 2/7] cobalt: posix/cond: Add missing __user annotation to user provided ptr

[Florian Bezdeka]

Like all other pointers in the cond_wait_prologue interface the error
pointer is user-provided and should be annotated accordingly.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 kernel/cobalt/posix/cond.c      | 8 ++++----
 kernel/cobalt/posix/cond.h      | 8 ++++----
 kernel/cobalt/posix/syscall32.c | 4 ++--
 kernel/cobalt/posix/syscall32.h | 4 ++--
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/kernel/cobalt/posix/cond.c b/kernel/cobalt/posix/cond.c
index 28400505f..e3a95ff83 100644
--- a/kernel/cobalt/posix/cond.c
+++ b/kernel/cobalt/posix/cond.c
@@ -285,7 +285,7 @@ static inline int cond_fetch_timeout64(struct timespec64 *ts,
 
 int __cobalt_cond_wait_prologue(struct cobalt_cond_shadow __user *u_cnd,
 				struct cobalt_mutex_shadow __user *u_mx,
-				int *u_err,
+				int __user *u_err,
 				void __user *u_ts,
 				int (*fetch_timeout)(struct timespec64 *ts,
 						     const void __user *u_ts))
@@ -351,7 +351,7 @@ int __cobalt_cond_wait_prologue(struct cobalt_cond_shadow __user *u_cnd,
 
 int __cobalt_cond_wait_prologue64(struct cobalt_cond_shadow __user *u_cnd,
 				  struct cobalt_mutex_shadow __user *u_mx,
-				  int *u_err, unsigned int timed,
+				  int __user *u_err, unsigned int timed,
 				  void __user *u_ts)
 {
 	return __cobalt_cond_wait_prologue(u_cnd, u_mx, u_err, u_ts,
@@ -362,7 +362,7 @@ int __cobalt_cond_wait_prologue64(struct cobalt_cond_shadow __user *u_cnd,
 COBALT_SYSCALL(cond_wait_prologue, nonrestartable,
 	       (struct cobalt_cond_shadow __user *u_cnd,
 		struct cobalt_mutex_shadow __user *u_mx,
-		int *u_err,
+		int __user *u_err,
 		unsigned int timed,
 		struct __user_old_timespec __user *u_ts))
 {
@@ -373,7 +373,7 @@ COBALT_SYSCALL(cond_wait_prologue, nonrestartable,
 COBALT_SYSCALL(cond_wait_prologue64, nonrestartable,
 	       (struct cobalt_cond_shadow __user *u_cnd,
 		struct cobalt_mutex_shadow __user *u_mx,
-		int *u_err,
+		int __user *u_err,
 		unsigned int timed,
 		struct __kernel_timespec __user *u_ts))
 {
diff --git a/kernel/cobalt/posix/cond.h b/kernel/cobalt/posix/cond.h
index b087b189e..1fd4256ce 100644
--- a/kernel/cobalt/posix/cond.h
+++ b/kernel/cobalt/posix/cond.h
@@ -41,14 +41,14 @@ struct cobalt_cond {
 
 int __cobalt_cond_wait_prologue(struct cobalt_cond_shadow __user *u_cnd,
 				struct cobalt_mutex_shadow __user *u_mx,
-				int *u_err,
+				int __user *u_err,
 				void __user *u_ts,
 				int (*fetch_timeout)(struct timespec64 *ts,
 						     const void __user *u_ts));
 
 int __cobalt_cond_wait_prologue64(struct cobalt_cond_shadow __user *u_cnd,
 				  struct cobalt_mutex_shadow __user *u_mx,
-				  int *u_err, unsigned int timed,
+				  int __user *u_err, unsigned int timed,
 				  void __user *u_ts);
 
 COBALT_SYSCALL_DECL(cond_init,
@@ -61,14 +61,14 @@ COBALT_SYSCALL_DECL(cond_destroy,
 COBALT_SYSCALL_DECL(cond_wait_prologue,
 		    (struct cobalt_cond_shadow __user *u_cnd,
 		     struct cobalt_mutex_shadow __user *u_mx,
-		     int *u_err,
+		     int __user *u_err,
 		     unsigned int timed,
 		     struct __user_old_timespec __user *u_ts));
 
 COBALT_SYSCALL_DECL(cond_wait_prologue64,
 		    (struct cobalt_cond_shadow __user *u_cnd,
 		     struct cobalt_mutex_shadow __user *u_mx,
-		     int *u_err,
+		     int __user *u_err,
 		     unsigned int timed,
 		     struct __kernel_timespec __user *u_ts));
 
diff --git a/kernel/cobalt/posix/syscall32.c b/kernel/cobalt/posix/syscall32.c
index fbd2b7f79..a6cf218ea 100644
--- a/kernel/cobalt/posix/syscall32.c
+++ b/kernel/cobalt/posix/syscall32.c
@@ -278,7 +278,7 @@ COBALT_SYSCALL32emu(mutex_timedlock64, primary,
 COBALT_SYSCALL32emu(cond_wait_prologue, nonrestartable,
 		    (struct cobalt_cond_shadow __user *u_cnd,
 		     struct cobalt_mutex_shadow __user *u_mx,
-		     int *u_err,
+		     int __user *u_err,
 		     unsigned int timed,
 		     struct old_timespec32 __user *u_ts))
 {
@@ -289,7 +289,7 @@ COBALT_SYSCALL32emu(cond_wait_prologue, nonrestartable,
 COBALT_SYSCALL32emu(cond_wait_prologue64, nonrestartable,
 		    (struct cobalt_cond_shadow __user *u_cnd,
 		     struct cobalt_mutex_shadow __user *u_mx,
-		     int *u_err,
+		     int __user *u_err,
 		     unsigned int timed,
 		     struct __kernel_timespec __user *u_ts))
 {
diff --git a/kernel/cobalt/posix/syscall32.h b/kernel/cobalt/posix/syscall32.h
index cdaa903ea..a64d100e7 100644
--- a/kernel/cobalt/posix/syscall32.h
+++ b/kernel/cobalt/posix/syscall32.h
@@ -105,14 +105,14 @@ COBALT_SYSCALL32emu_DECL(mutex_timedlock64,
 COBALT_SYSCALL32emu_DECL(cond_wait_prologue,
 			 (struct cobalt_cond_shadow __user *u_cnd,
 			  struct cobalt_mutex_shadow __user *u_mx,
-			  int *u_err,
+			  int __user *u_err,
 			  unsigned int timed,
 			  struct old_timespec32 __user *u_ts));
 
 COBALT_SYSCALL32emu_DECL(cond_wait_prologue64,
 			 (struct cobalt_cond_shadow __user *u_cnd,
 			  struct cobalt_mutex_shadow __user *u_mx,
-			  int *u_err,
+			  int __user *u_err,
 			  unsigned int timed,
 			  struct __kernel_timespec __user *u_ts));
 
-- 
2.30.2

[PATCH 3/7] y2038: lib/cobalt: Dispatch cond_wait_prologue

[Florian Bezdeka]

It libc reports time64_t support, cond_wait_prologue is now dispatched
to the time64_t based syscall.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 lib/cobalt/cond.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/lib/cobalt/cond.c b/lib/cobalt/cond.c
index 1bf5c74b3..35b367a73 100644
--- a/lib/cobalt/cond.c
+++ b/lib/cobalt/cond.c
@@ -227,6 +227,20 @@ static void __pthread_cond_cleanup(void *data)
 	c->mutex->lockcnt = c->count;
 }
 
+static inline int do_sc_cond_wait_prologue(struct cobalt_cond_shadow *cnd,
+					   struct cobalt_mutex_shadow *mx,
+					   int *err, int timed,
+					   const struct timespec *abstime)
+{
+#ifdef __USE_TIME_BITS64
+	long sc_nr = sc_cobalt_cond_wait_prologue64;
+#else
+	long sc_nr = sc_cobalt_cond_wait_prologue;
+#endif
+
+	return XENOMAI_SYSCALL5(sc_nr, cnd, mx, err, timed, abstime);
+}
+
 /**
  * Wait on a condition variable.
  *
@@ -310,8 +324,7 @@ COBALT_IMPL(int, pthread_cond_wait, (pthread_cond_t *cond, pthread_mutex_t *mute
 
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, &oldtype);
 
-	err = XENOMAI_SYSCALL5(sc_cobalt_cond_wait_prologue,
-			       _cnd, _mx, &c.err, 0, NULL);
+	err = do_sc_cond_wait_prologue(_cnd, _mx, &c.err, 0, NULL);
 
 	pthread_setcanceltype(oldtype, NULL);
 
@@ -399,8 +412,8 @@ COBALT_IMPL(int, pthread_cond_timedwait, (pthread_cond_t *cond,
 
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, &oldtype);
 
-	err = XENOMAI_SYSCALL5(sc_cobalt_cond_wait_prologue,
-			       _cnd, _mx, &c.err, 1, abstime);
+	err = do_sc_cond_wait_prologue(_cnd, _mx, &c.err, 1, abstime);
+
 	pthread_setcanceltype(oldtype, NULL);
 
 	pthread_cleanup_pop(0);
-- 
2.30.2

[PATCH 4/7] cobalt: posix/cond: Add missing input validations

[Florian Bezdeka]

The following validation issues have been addressed:

  - __cobalt_cond_wait_prologue() missed validating the supplied
    pointers after the registry lookup which could fail. That triggered
    the kernel OOPS dumped below

  - The check removed from cobalt_cond_timedwait_prologue() is now
    already done in __cobalt_cond_wait_prologue()

  - The entry point for the cond_wait_epilogue syscall missed the same
    validations

  - __cobalt_cond_wait_prologue() missed the validation for the
    supplied timeout

[   21.254929] BUG: kernel NULL pointer dereference, address: 0000000000000078
[   21.254930] #PF: supervisor read access in kernel mode
[   21.254931] #PF: error_code(0x0000) - not-present page
[   21.254932] PGD 0 P4D 0
[   21.254933] Oops: 0000 [#1] SMP NOPTI IRQ_PIPELINE
[   21.254934] CPU: 1 PID: 271 Comm: smokey Not tainted 5.10.76+ #54
[   21.254935] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014
[   21.254935] IRQ stage: Linux
[   21.254936] RIP: 0010:__cobalt_cond_wait_prologue+0x28c/0x430
[   21.254937] Code: 48 d1 e8 83 e0 01 49 39 c5 0f 85 10 01 00 00 48 83 7b 78 00 45 89 f8 44 89 f8 45 89 e7 0f 85 ed 00 00 00b
[   21.254937] RSP: 0018:ffffc90000843e30 EFLAGS: 00010246
[   21.254938] RAX: fffffffffffffff2 RBX: 0000000000000000 RCX: 0000000000000000
[   21.254938] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
[   21.254938] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
[   21.254939] R10: 0000000000019000 R11: 0000000000000000 R12: ffffc9000061c408
[   21.254939] R13: 0000000000000000 R14: 0000000000000000 R15: ffffffff8118f400
[   21.254939] FS:  00007fdfe9f79680(0000) GS:ffff88803e880000(0000) knlGS:0000000000000000
[   21.254939] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   21.254940] CR2: 0000000000000078 CR3: 0000000004108000 CR4: 00000000003506e0
[   21.254940] Call Trace:
[   21.254940]  ? dovetail_leave_inband+0xdc/0x120
[   21.254940]  ? CoBaLt_cond_wait_prologue+0x30/0x30
[   21.254941]  CoBaLt_cond_wait_prologue64+0x1e/0x30
[   21.254941]  handle_root_syscall+0xe0/0x2d0
[   21.254941]  __pipeline_syscall+0xb3/0x230
[   21.254941]  ? vfs_write+0x14d/0x270
[   21.254942]  pipeline_syscall+0x33/0xe0
[   21.254942]  syscall_enter_from_user_mode+0x23/0x80
[   21.254942]  do_syscall_64+0xf/0x50
[   21.254942]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[   21.254943] RIP: 0033:0x5617e786b83d
[   21.254943] Code: 00 00 00 00 48 c7 45 c8 00 00 00 00 4c 8b 45 e8 4c 8b 55 e0 48 8b 55 d8 48 8b 75 d0 48 8b 7d c8 8b 45 f05
[   21.254943] RSP: 002b:00007ffd22b84db0 EFLAGS: 00000202 ORIG_RAX: 0000000010000073
[   21.254944] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00005617e786b83d
[   21.254944] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
[   21.254945] RBP: 00007ffd22b84f40 R08: 0000000000000000 R09: 0000000000000001
[   21.254945] R10: 0000000000000000 R11: 0000000000000202 R12: 00005617e7848e60
[   21.254945] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[   21.254946] Modules linked in:
[   21.254946] CR2: 0000000000000078
[   21.254946] ---[ end trace 4f2931a73a5a875d ]---
[   21.254946] RIP: 0010:__cobalt_cond_wait_prologue+0x28c/0x430
[   21.254947] Code: 48 d1 e8 83 e0 01 49 39 c5 0f 85 10 01 00 00 48 83 7b 78 00 45 89 f8 44 89 f8 45 89 e7 0f 85 ed 00 00 00b
[   21.254947] RSP: 0018:ffffc90000843e30 EFLAGS: 00010246
[   21.254948] RAX: fffffffffffffff2 RBX: 0000000000000000 RCX: 0000000000000000
[   21.254948] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
[   21.254948] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
[   21.254949] R10: 0000000000019000 R11: 0000000000000000 R12: ffffc9000061c408
[   21.254949] R13: 0000000000000000 R14: 0000000000000000 R15: ffffffff8118f400
[   21.254949] FS:  00007fdfe9f79680(0000) GS:ffff88803e880000(0000) knlGS:0000000000000000
[   21.254950] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   21.254950] CR2: 0000000000000078 CR3: 0000000004108000 CR4: 00000000003506e0

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 kernel/cobalt/posix/cond.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/kernel/cobalt/posix/cond.c b/kernel/cobalt/posix/cond.c
index e3a95ff83..95d1e75bc 100644
--- a/kernel/cobalt/posix/cond.c
+++ b/kernel/cobalt/posix/cond.c
@@ -134,8 +134,7 @@ static inline int cobalt_cond_timedwait_prologue(struct xnthread *cur,
 	xnlock_get_irqsave(&nklock, s);
 
 	/* If another thread waiting for cond does not use the same mutex */
-	if (!cobalt_obj_active(cond, COBALT_COND_MAGIC, struct cobalt_cond)
-	    || (cond->mutex && cond->mutex != mutex)) {
+	if ((cond->mutex && cond->mutex != mutex)) {
 		err = -EINVAL;
 		goto unlock_and_return;
 	}
@@ -301,9 +300,13 @@ int __cobalt_cond_wait_prologue(struct cobalt_cond_shadow __user *u_cnd,
 
 	handle = cobalt_get_handle_from_user(&u_cnd->handle);
 	cond = xnregistry_lookup(handle, NULL);
+	if (!cobalt_obj_active(cond, COBALT_COND_MAGIC, typeof(*cond)))
+		return -EINVAL;
 
 	handle = cobalt_get_handle_from_user(&u_mx->handle);
 	mx = xnregistry_lookup(handle, NULL);
+	if (!cobalt_obj_active(mx, COBALT_MUTEX_MAGIC, typeof(*mx)))
+		return -EINVAL;
 
 	if (cond->mutex == NULL) {
 		__xn_get_user(offset, &u_mx->state_offset);
@@ -313,9 +316,12 @@ int __cobalt_cond_wait_prologue(struct cobalt_cond_shadow __user *u_cnd,
 	if (fetch_timeout) {
 		err = fetch_timeout(&ts, u_ts);
 		if (err == 0) {
-			trace_cobalt_cond_timedwait(u_cnd, u_mx, &ts);
-			err = cobalt_cond_timedwait_prologue(cur, cond, mx,
-							     ts2ns(&ts) + 1);
+			if (timespec64_valid(&ts)) {
+				trace_cobalt_cond_timedwait(u_cnd, u_mx, &ts);
+				err = cobalt_cond_timedwait_prologue(
+					cur, cond, mx, ts2ns(&ts) + 1);
+			} else
+				err = -EINVAL;
 		}
 	} else {
 		trace_cobalt_cond_wait(u_cnd, u_mx);
@@ -334,9 +340,8 @@ int __cobalt_cond_wait_prologue(struct cobalt_cond_shadow __user *u_cnd,
 		d.err = 0;	/* epilogue should return 0. */
 		break;
 
+	case -EINVAL:
 	default:
-		/* Please gcc and handle the case which will never
-		   happen */
 		d.err = EINVAL;
 	}
 
@@ -392,9 +397,14 @@ COBALT_SYSCALL(cond_wait_epilogue, primary,
 
 	handle = cobalt_get_handle_from_user(&u_cnd->handle);
 	cond = xnregistry_lookup(handle, NULL);
+	if (!cobalt_obj_active(cond, COBALT_COND_MAGIC, typeof(*cond)))
+		return -EINVAL;
 
 	handle = cobalt_get_handle_from_user(&u_mx->handle);
 	mx = xnregistry_lookup(handle, NULL);
+	if (!cobalt_obj_active(mx, COBALT_MUTEX_MAGIC, typeof(*mx)))
+		return -EINVAL;
+
 	err = cobalt_cond_timedwait_epilogue(cur, cond, mx);
 
 	if (cond->mutex == NULL)
-- 
2.30.2

[PATCH 5/7] y2038: testsuite/smokey/y2038: Adding tests for cond_wait_prologue64

[Florian Bezdeka]

Extending the smokey testsuite to do some tests for the recently added
cond_wait_prologue64 syscall.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 testsuite/smokey/y2038/syscall-tests.c | 97 ++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/testsuite/smokey/y2038/syscall-tests.c b/testsuite/smokey/y2038/syscall-tests.c
index a1a5d12f6..874decc2a 100644
--- a/testsuite/smokey/y2038/syscall-tests.c
+++ b/testsuite/smokey/y2038/syscall-tests.c
@@ -1116,6 +1116,99 @@ out:
 	return ret;
 }
 
+static int test_sc_cobalt_cond_wait_prologue(void)
+{
+	int ret = 0;
+	int err = 0;
+	int sc_nr = sc_cobalt_cond_wait_prologue64;
+	pthread_mutex_t m;
+	pthread_cond_t c;
+	pthread_condattr_t attr;
+	struct xn_timespec64 t1, t2;
+	struct timespec ts_nat;
+
+	if (!__T(ret, pthread_mutex_init(&m, NULL)))
+		return ret;
+
+	if (!__T(ret, pthread_condattr_init(&attr)))
+		goto out_mutex;
+
+	if (!__T(ret, pthread_cond_init(&c, &attr)))
+		goto out_cond_attr;
+
+	/* Make sure we don't crash because of NULL pointers */
+	ret = XENOMAI_SYSCALL5(sc_nr, NULL, NULL, NULL, NULL, NULL);
+	if (ret == -ENOSYS) {
+		smokey_note(
+			"cond_wait_prologue64: skipped. (no kernel support)");
+		return 0; // Not implemented, nothing to test, success
+	}
+	if (!smokey_assert(ret == -EINVAL))
+		return ret ? ret : -EINVAL;
+
+	/* Timed, but no timeout supplied, should deliver EFAULT */
+	ret = XENOMAI_SYSCALL5(sc_nr, &c, &m, &err, 1 /* timed */, NULL);
+	if (!smokey_assert(ret == -EFAULT)) {
+		ret = ret ? ret : -EINVAL;
+		goto out;
+	}
+
+	/* Timed and invalid timeout supplied, should deliver EINVAL */
+	t1.tv_sec = -1;
+	t1.tv_nsec = 0;
+	ret = XENOMAI_SYSCALL5(sc_nr, &c, &m, &err, 1 /* timed */, &t1);
+	if (!smokey_assert(ret == -EINVAL)) {
+		ret = ret ? ret : -EINVAL;
+		goto out;
+	}
+
+	/*
+	 * Providing a valid timeout, waiting for it to time out and check
+	 * that we didn't come back to early.
+	 */
+	ret = smokey_check_errno(clock_gettime(CLOCK_MONOTONIC, &ts_nat));
+	if (ret)
+		goto out;
+
+	t1.tv_sec = 0;
+	t1.tv_nsec = 500000;
+
+	if (!__T(ret, pthread_mutex_lock(&m)))
+		goto out;
+
+	ret = XENOMAI_SYSCALL5(sc_nr, &c, &m, &err, 1 /* timed */, &t1);
+	if (!smokey_assert(ret == -ETIMEDOUT)) {
+		ret = ret ? ret : -EINVAL;
+		goto out;
+	}
+
+	t1.tv_sec = ts_nat.tv_sec;
+	t1.tv_nsec = ts_nat.tv_nsec;
+
+	ret = smokey_check_errno(clock_gettime(CLOCK_MONOTONIC, &ts_nat));
+	if (ret)
+		goto out;
+
+	t2.tv_sec = ts_nat.tv_sec;
+	t2.tv_nsec = ts_nat.tv_nsec;
+
+	if (ts_less(&t2, &t1))
+		smokey_warning("cond_wait_prologue64 returned to early!\n"
+			       "Expected wakeup at: %lld sec %lld nsec\n"
+			       "Back at           : %lld sec %lld nsec\n",
+			       t1.tv_sec, t1.tv_nsec, t2.tv_sec, t2.tv_nsec);
+
+	pthread_mutex_unlock(&m);
+
+out:
+	pthread_cond_destroy(&c);
+out_cond_attr:
+	pthread_condattr_destroy(&attr);
+out_mutex:
+	pthread_mutex_destroy(&m);
+
+	return ret;
+}
 
 static int check_kernel_version(void)
 {
@@ -1199,5 +1292,9 @@ static int run_y2038(struct smokey_test *t, int argc, char *const argv[])
 	if (ret)
 		return ret;
 
+	ret = test_sc_cobalt_cond_wait_prologue();
+	if (ret)
+		return ret;
+
 	return 0;
 }
-- 
2.30.2

[PATCH 6/7] y2038: testsuite/smokey/y2038: Add a missing error handling path

[Florian Bezdeka]

Initialization of the mutex used for mutex_timedlock64 tests could fail.
We have to abort the test in this case.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 testsuite/smokey/y2038/syscall-tests.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/testsuite/smokey/y2038/syscall-tests.c b/testsuite/smokey/y2038/syscall-tests.c
index 874decc2a..2f4041e51 100644
--- a/testsuite/smokey/y2038/syscall-tests.c
+++ b/testsuite/smokey/y2038/syscall-tests.c
@@ -489,7 +489,8 @@ static int test_sc_cobalt_mutex_timedlock64(void)
 	struct xn_timespec64 ts64;
 	struct thread_context ctx = {0};
 
-	ret = pthread_mutex_init(&mutex, NULL);
+	if (!__T(ret, pthread_mutex_init(&mutex, NULL)))
+		return ret;
 
 	/* Make sure we don't crash because of NULL pointers */
 	ret = XENOMAI_SYSCALL2(sc_nr, NULL, NULL);
-- 
2.30.2

[PATCH 7/7] cobalt: Protect __xn_get_user() by access_ok()

[Florian Bezdeka]

According to the doctype provided by __get_user (which is used by the
__xn_get_user() macro) each call should be protected by access_ok().
We missed such a protection at some places.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 kernel/cobalt/posix/internal.h  | 4 ++++
 kernel/cobalt/posix/nsem.c      | 3 ++-
 kernel/cobalt/posix/syscall32.c | 3 ++-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/kernel/cobalt/posix/internal.h b/kernel/cobalt/posix/internal.h
index 8b134d0a7..36fbc9eb5 100644
--- a/kernel/cobalt/posix/internal.h
+++ b/kernel/cobalt/posix/internal.h
@@ -52,6 +52,10 @@ extern struct xnptree posix_ptree;
 static inline xnhandle_t cobalt_get_handle_from_user(xnhandle_t *u_h)
 {
 	xnhandle_t handle;
+
+	if (unlikely(!access_ok(u_h, sizeof(*u_h))))
+		return 0;
+
 	return __xn_get_user(handle, u_h) ? 0 : handle;
 }
 
diff --git a/kernel/cobalt/posix/nsem.c b/kernel/cobalt/posix/nsem.c
index 89cf62b6f..a6481c092 100644
--- a/kernel/cobalt/posix/nsem.c
+++ b/kernel/cobalt/posix/nsem.c
@@ -222,7 +222,8 @@ COBALT_SYSCALL(sem_open, lostage,
 {
 	struct cobalt_sem_shadow __user *usm;
 
-	if (__xn_get_user(usm, u_addrp))
+	if (!access_ok(u_addrp, sizeof(*u_addrp)) ||
+	    __xn_get_user(usm, u_addrp))
 		return -EFAULT;
 
 	usm = __cobalt_sem_open(usm, u_name, oflags, mode, value);
diff --git a/kernel/cobalt/posix/syscall32.c b/kernel/cobalt/posix/syscall32.c
index a6cf218ea..0c3f50a81 100644
--- a/kernel/cobalt/posix/syscall32.c
+++ b/kernel/cobalt/posix/syscall32.c
@@ -113,7 +113,8 @@ COBALT_SYSCALL32emu(sem_open, lostage,
 	struct cobalt_sem_shadow __user *usm;
 	compat_uptr_t cusm;
 
-	if (__xn_get_user(cusm, u_addrp))
+	if (!access_ok(u_addrp, sizeof(*u_addrp)) ||
+	    __xn_get_user(cusm, u_addrp))
 		return -EFAULT;
 
 	usm = __cobalt_sem_open(compat_ptr(cusm), u_name, oflags, mode, value);
-- 
2.30.2

Re: [PATCH 0/7] y2038: cond_wait_prologue64 and related fixes

[Jan Kiszka]

On 05.01.22 15:06, Florian Bezdeka wrote:
> Hi all,
> 
> this is the last missing POSIX related y2038 affected syscall in
> Xenomai. With this applied we have two Xenomai specific syscalls
> missing:
> 
>   - sc_cobalt_thread_setschedparam_ex
>   - sc_cobalt_thread_getschedparam_ex
> 
> While adding tests for the introduced cond_wait_prologue64 I hit a
> kernel OOPS due to insuficient validation of user provided pointers.
> That has been addressed as well.

Thanks for both! Is it possibly to move the fixes the front? That would
also ensure that I can easily pick them into stable.

Thanks,
Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux

Re: [PATCH 0/7] y2038: cond_wait_prologue64 and related fixes

[Bezdeka, Florian]

On Wed, 2022-01-05 at 15:43 +0100, Jan Kiszka wrote:
> On 05.01.22 15:06, Florian Bezdeka wrote:
> > Hi all,
> > 
> > this is the last missing POSIX related y2038 affected syscall in
> > Xenomai. With this applied we have two Xenomai specific syscalls
> > missing:
> > 
> >   - sc_cobalt_thread_setschedparam_ex
> >   - sc_cobalt_thread_getschedparam_ex
> > 
> > While adding tests for the introduced cond_wait_prologue64 I hit a
> > kernel OOPS due to insuficient validation of user provided pointers.
> > That has been addressed as well.
> 
> Thanks for both! Is it possibly to move the fixes the front? That would
> also ensure that I can easily pick them into stable.

Yes. Patch 4 and 7 could be moved to the front easily. Do you want me
to split patch 2 into the y2038 and non y2038 part, or does that not
qualify for stable at all?

> 
> Thanks,
> Jan
> 

Re: [PATCH 0/7] y2038: cond_wait_prologue64 and related fixes

[Jan Kiszka]

On 05.01.22 15:56, Bezdeka, Florian (T CED SES-DE) wrote:
> On Wed, 2022-01-05 at 15:43 +0100, Jan Kiszka wrote:
>> On 05.01.22 15:06, Florian Bezdeka wrote:
>>> Hi all,
>>>
>>> this is the last missing POSIX related y2038 affected syscall in
>>> Xenomai. With this applied we have two Xenomai specific syscalls
>>> missing:
>>>
>>>   - sc_cobalt_thread_setschedparam_ex
>>>   - sc_cobalt_thread_getschedparam_ex
>>>
>>> While adding tests for the introduced cond_wait_prologue64 I hit a
>>> kernel OOPS due to insuficient validation of user provided pointers.
>>> That has been addressed as well.
>>
>> Thanks for both! Is it possibly to move the fixes the front? That would
>> also ensure that I can easily pick them into stable.
> 
> Yes. Patch 4 and 7 could be moved to the front easily. Do you want me
> to split patch 2 into the y2038 and non y2038 part, or does that not
> qualify for stable at all?

Can I reorder things myself, or does patch 4 break (patch 7 does not,
already checked)? Then I just change the application order while doing
git am.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux

Re: [PATCH 0/7] y2038: cond_wait_prologue64 and related fixes

[Bezdeka, Florian]

On Wed, 2022-01-05 at 15:58 +0100, Jan Kiszka wrote:
> On 05.01.22 15:56, Bezdeka, Florian (T CED SES-DE) wrote:
> > On Wed, 2022-01-05 at 15:43 +0100, Jan Kiszka wrote:
> > > On 05.01.22 15:06, Florian Bezdeka wrote:
> > > > Hi all,
> > > > 
> > > > this is the last missing POSIX related y2038 affected syscall in
> > > > Xenomai. With this applied we have two Xenomai specific syscalls
> > > > missing:
> > > > 
> > > >   - sc_cobalt_thread_setschedparam_ex
> > > >   - sc_cobalt_thread_getschedparam_ex
> > > > 
> > > > While adding tests for the introduced cond_wait_prologue64 I hit a
> > > > kernel OOPS due to insuficient validation of user provided pointers.
> > > > That has been addressed as well.
> > > 
> > > Thanks for both! Is it possibly to move the fixes the front? That would
> > > also ensure that I can easily pick them into stable.
> > 
> > Yes. Patch 4 and 7 could be moved to the front easily. Do you want me
> > to split patch 2 into the y2038 and non y2038 part, or does that not
> > qualify for stable at all?
> 
> Can I reorder things myself, or does patch 4 break (patch 7 does not,
> already checked)? Then I just change the application order while doing
> git am.

No breakage expected. The only "problematic" one would be patch 2 as it
touches y2038 as well as non-y2038 syscall definitions. Let me know if
I should split that into two parts (which would allow the non y2038
related cleanup to be applied to stable separately)

> 
> Jan
> 

Re: [PATCH 0/7] y2038: cond_wait_prologue64 and related fixes

[Jan Kiszka]

On 05.01.22 16:08, Bezdeka, Florian (T CED SES-DE) wrote:
> On Wed, 2022-01-05 at 15:58 +0100, Jan Kiszka wrote:
>> On 05.01.22 15:56, Bezdeka, Florian (T CED SES-DE) wrote:
>>> On Wed, 2022-01-05 at 15:43 +0100, Jan Kiszka wrote:
>>>> On 05.01.22 15:06, Florian Bezdeka wrote:
>>>>> Hi all,
>>>>>
>>>>> this is the last missing POSIX related y2038 affected syscall in
>>>>> Xenomai. With this applied we have two Xenomai specific syscalls
>>>>> missing:
>>>>>
>>>>>   - sc_cobalt_thread_setschedparam_ex
>>>>>   - sc_cobalt_thread_getschedparam_ex
>>>>>
>>>>> While adding tests for the introduced cond_wait_prologue64 I hit a
>>>>> kernel OOPS due to insuficient validation of user provided pointers.
>>>>> That has been addressed as well.
>>>>
>>>> Thanks for both! Is it possibly to move the fixes the front? That would
>>>> also ensure that I can easily pick them into stable.
>>>
>>> Yes. Patch 4 and 7 could be moved to the front easily. Do you want me
>>> to split patch 2 into the y2038 and non y2038 part, or does that not
>>> qualify for stable at all?
>>
>> Can I reorder things myself, or does patch 4 break (patch 7 does not,
>> already checked)? Then I just change the application order while doing
>> git am.
> 
> No breakage expected. The only "problematic" one would be patch 2 as it
> touches y2038 as well as non-y2038 syscall definitions. Let me know if
> I should split that into two parts (which would allow the non y2038
> related cleanup to be applied to stable separately)

The annotation patch is not needed for stable.

I'm applying now 4 7 1 2 3 5 6 and will kick off testing.

Thanks,
Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux