xenomai.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] y2038: Part three - support for select()
@ 2023-05-30 18:57 Florian Bezdeka
  2023-05-30 18:57 ` [PATCH v3 1/4] y2038: cobalt/posix/select: Refactor __cobalt_select() Florian Bezdeka
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Florian Bezdeka @ 2023-05-30 18:57 UTC (permalink / raw)
  To: jan.kiszka, xenomai; +Cc: Florian Bezdeka

Hi all,

I'm trying to bring the remaining patches from my y2038 queue into Xenomai 
next/master branches. The full queue [1] holds ~7 patches. I'm trying to 
split that up to keep reviewing efforts low.

This series brings y2038 support for select().

select() is somehow special. Linux does not support a y2038 safe
implementation for select() itself. If the application needs y2038
support the application has to use something like the pselect() service.

To make select() y2038 safe glibc wrapps select() and wires it to the 
pselect() Linux service.

Xenomai (cobalt) does not provide pselect() yet and it's not easy to 
implement that. The reasons:

  - The underlying xnsynch infrastructure is not prepared for waiting for a
    combination of file descriptors and signals. (Let me know if I
    overlooked something!)

  - Implementing pselect() would require up to 6 parameters that need to
    be forwarded to the kernel. Xenomais current limit is 5.

To get select() y2038 safe a "pselect64() like" service service is being
added. The signal part is skipped.

The wrapper for __select64() will be added later and will convert 
struct timeval to struct timespec64 in user space. This way select() 
provided by libcobalt gets y2038 support but pselect() is still not 
supported.

Best regards,
Florian

[1] https://gitlab.com/Xenomai/xenomai-hacker-space/-/tree/florian/y2038

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
Changes in v3:
- Patch 1: Remove trailing comments, document error handling 
- Patch 2: Remove trailing comment, document error handling
- Patch 4: Rewrite commit message
- Link to v2: https://lore.kernel.org/r/20230516-florian-y2038-part-three-v2-0-3fc689b21a32@siemens.com

Changes in v2:
- Patch 1: Do not skip updating the timeout if fds changed
           Fix is in all entry points.
- Patch 2: pselect64: Same as above. Do not skip the update if fds
           changed.
- Patch 3: unmodified
- Patch 4: New patch. Fix timeout update in case of -EINTR

- Link to v1: https://lore.kernel.org/r/20230516-florian-y2038-part-three-v1-0-b140278b26c6@siemens.com

---
Florian Bezdeka (4):
      y2038: cobalt/posix/select: Refactor __cobalt_select()
      y2038: cobalt/posix/select: Adding pselect64
      y2038: testsuite/smokey/y2038: Adding tests for pselect64
      cobalt/posix/select: Fix timeout update in case of -EINTR

 include/cobalt/uapi/syscall.h                      |   1 +
 .../cobalt/include/asm-generic/xenomai/syscall.h   |  22 +++
 kernel/cobalt/posix/clock.h                        |   6 +-
 kernel/cobalt/posix/io.c                           | 109 +++++++++-----
 kernel/cobalt/posix/io.h                           |   8 +-
 kernel/cobalt/posix/syscall32.c                    |  23 ++-
 kernel/cobalt/trace/cobalt-posix.h                 |   3 +-
 testsuite/smokey/y2038/syscall-tests.c             | 162 +++++++++++++++++++++
 8 files changed, 290 insertions(+), 44 deletions(-)
---
base-commit: a74abe52269f191654b449357bd9aaa57f2cd75c
change-id: 20230516-florian-y2038-part-three-3093da9fecf8

Best regards,
-- 
Florian Bezdeka <florian.bezdeka@siemens.com>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/4] y2038: cobalt/posix/select: Refactor __cobalt_select()
  2023-05-30 18:57 [PATCH v3 0/4] y2038: Part three - support for select() Florian Bezdeka
@ 2023-05-30 18:57 ` Florian Bezdeka
  2023-05-30 18:57 ` [PATCH v3 2/4] y2038: cobalt/posix/select: Adding pselect64 Florian Bezdeka
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Florian Bezdeka @ 2023-05-30 18:57 UTC (permalink / raw)
  To: jan.kiszka, xenomai; +Cc: Florian Bezdeka

There is no y2038 safe select() syscall provided by Linuy. If y2038
safety is required the application should not use select() and move on
to something like pselect().

As we still want to support select() for keeping backward compatibility
- even if y2038 safety is requested - an additional pselect() based
entry will be introduced soon which will re-use __cobalt_select().

This patch moves all the timeout related userspace copy operations out
of __cobalt_select() and migrates __cobalt_select() to
struct timespec64. That will allow pselect64() to be based on
__cobalt_select() as well.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 .../cobalt/include/asm-generic/xenomai/syscall.h   | 22 ++++++++
 kernel/cobalt/posix/clock.h                        |  6 +-
 kernel/cobalt/posix/io.c                           | 65 +++++++++++-----------
 kernel/cobalt/posix/syscall32.c                    | 23 +++++++-
 4 files changed, 80 insertions(+), 36 deletions(-)

diff --git a/kernel/cobalt/include/asm-generic/xenomai/syscall.h b/kernel/cobalt/include/asm-generic/xenomai/syscall.h
index 70b3e68f9..d61b985fb 100644
--- a/kernel/cobalt/include/asm-generic/xenomai/syscall.h
+++ b/kernel/cobalt/include/asm-generic/xenomai/syscall.h
@@ -138,6 +138,28 @@ static inline int cobalt_put_u_itimerspec(
 	return cobalt_copy_to_user(dst, &u_its, sizeof(*dst));
 }
 
+static inline struct timespec64
+cobalt_timeval_to_timespec64(const struct __kernel_old_timeval *src)
+{
+	struct timespec64 ts;
+
+	ts.tv_sec = src->tv_sec + (src->tv_usec / USEC_PER_SEC);
+	ts.tv_nsec = (src->tv_usec % USEC_PER_SEC) * NSEC_PER_USEC;
+
+	return ts;
+}
+
+static inline struct __kernel_old_timeval
+cobalt_timespec64_to_timeval(const struct timespec64 *src)
+{
+	struct __kernel_old_timeval tv;
+
+	tv.tv_sec = src->tv_sec + (src->tv_nsec / NSEC_PER_SEC);
+	tv.tv_usec = (src->tv_nsec % NSEC_PER_SEC) / NSEC_PER_USEC;
+
+	return tv;
+}
+
 /* 32bit syscall emulation */
 #define __COBALT_COMPAT_BIT	0x1
 /* 32bit syscall emulation - extended form */
diff --git a/kernel/cobalt/posix/clock.h b/kernel/cobalt/posix/clock.h
index dbaabbc5d..38b59e24f 100644
--- a/kernel/cobalt/posix/clock.h
+++ b/kernel/cobalt/posix/clock.h
@@ -68,12 +68,12 @@ static inline xnticks_t tv2ns(const struct __kernel_old_timeval *tv)
 	return nsecs;
 }
 
-static inline void ticks2tv(struct __kernel_old_timeval *tv, xnticks_t ticks)
+static inline void ticks2ts64(struct timespec64 *ts, xnticks_t ticks)
 {
 	unsigned long nsecs;
 
-	tv->tv_sec = xnclock_divrem_billion(ticks, &nsecs);
-	tv->tv_usec = nsecs / 1000;
+	ts->tv_sec = xnclock_divrem_billion(ticks, &nsecs);
+	ts->tv_nsec = nsecs;
 }
 
 static inline xnticks_t clock_get_ticks(clockid_t clock_id)
diff --git a/kernel/cobalt/posix/io.c b/kernel/cobalt/posix/io.c
index 45ec09fae..786cb8ea9 100644
--- a/kernel/cobalt/posix/io.c
+++ b/kernel/cobalt/posix/io.c
@@ -221,7 +221,7 @@ static int __cobalt_select_bind_all(struct xnselector *selector,
 }
 
 int __cobalt_select(int nfds, void __user *u_rfds, void __user *u_wfds,
-		    void __user *u_xfds, void __user *u_tv, bool compat)
+		    void __user *u_xfds, struct timespec64 *to, bool compat)
 {
 	void __user *ufd_sets[XNSELECT_MAX_TYPES] = {
 		[XNSELECT_READ] = u_rfds,
@@ -237,13 +237,12 @@ int __cobalt_select(int nfds, void __user *u_rfds, void __user *u_wfds,
 	xntmode_t mode = XN_RELATIVE;
 	struct xnselector *selector;
 	struct xnthread *curr;
-	struct __kernel_old_timeval tv;
 	size_t fds_size;
 	int i, err;
 
 	curr = xnthread_current();
 
-	if (u_tv) {
+	if (to) {
 		if (xnthread_test_localinfo(curr, XNSYSRST)) {
 			xnthread_clear_localinfo(curr, XNSYSRST);
 
@@ -255,23 +254,11 @@ int __cobalt_select(int nfds, void __user *u_rfds, void __user *u_wfds,
 				goto out;
 			}
 		} else {
-#ifdef CONFIG_XENO_ARCH_SYS3264
-			if (compat) {
-				if (sys32_get_timeval(&tv, u_tv))
-					return -EFAULT;
-			} else
-#endif
-			{
-				if (!access_ok(u_tv, sizeof(tv))
-				    || cobalt_copy_from_user(&tv, u_tv,
-							     sizeof(tv)))
-					return -EFAULT;
-			}
 
-			if (tv.tv_usec >= 1000000)
+			if (!timespec64_valid(to))
 				return -EINVAL;
 
-			timeout = clock_get_ticks(CLOCK_MONOTONIC) + tv2ns(&tv);
+			timeout = clock_get_ticks(CLOCK_MONOTONIC) + ts2ns(to);
 		}
 
 		mode = XN_ABSOLUTE;
@@ -343,23 +330,12 @@ int __cobalt_select(int nfds, void __user *u_rfds, void __user *u_wfds,
 	}
 
 out:
-	if (u_tv && (err > 0 || err == -EINTR)) {
+	if (to && (err > 0 || err == -EINTR)) {
 		xnsticks_t diff = timeout - clock_get_ticks(CLOCK_MONOTONIC);
 		if (diff > 0)
-			ticks2tv(&tv, diff);
+			ticks2ts64(to, diff);
 		else
-			tv.tv_sec = tv.tv_usec = 0;
-
-#ifdef CONFIG_XENO_ARCH_SYS3264
-		if (compat) {
-			if (sys32_put_timeval(u_tv, &tv))
-				return -EFAULT;
-		} else
-#endif
-		{
-			if (cobalt_copy_to_user(u_tv, &tv, sizeof(tv)))
-				return -EFAULT;
-		}
+			to->tv_sec = to->tv_nsec = 0;
 	}
 
 	if (err >= 0)
@@ -390,5 +366,30 @@ COBALT_SYSCALL(select, primary,
 		fd_set __user *u_xfds,
 		struct __kernel_old_timeval __user *u_tv))
 {
-	return __cobalt_select(nfds, u_rfds, u_wfds, u_xfds, u_tv, false);
+	struct timespec64 ts64, *to = NULL;
+	struct __kernel_old_timeval tv;
+	int ret;
+
+	if (u_tv && (!access_ok(u_tv, sizeof(tv)) ||
+		     cobalt_copy_from_user(&tv, u_tv, sizeof(tv))))
+		return -EFAULT;
+
+	if (u_tv) {
+		ts64 = cobalt_timeval_to_timespec64(&tv);
+		to = &ts64;
+	}
+
+	ret = __cobalt_select(nfds, u_rfds, u_wfds, u_xfds, to, false);
+
+	if (u_tv) {
+		tv = cobalt_timespec64_to_timeval(to);
+		/*
+		 * Writing to u_tv might fail. We ignore a possible error here
+		 * to report back the number of changed fds (or the error).
+		 * This is aligned with the Linux select() implementation.
+		 */
+		cobalt_copy_to_user(u_tv, &tv, sizeof(tv));
+	}
+
+	return ret;
 }
diff --git a/kernel/cobalt/posix/syscall32.c b/kernel/cobalt/posix/syscall32.c
index 10b080dfd..780d276b1 100644
--- a/kernel/cobalt/posix/syscall32.c
+++ b/kernel/cobalt/posix/syscall32.c
@@ -749,7 +749,28 @@ COBALT_SYSCALL32emu(select, primary,
 		     compat_fd_set __user *u_xfds,
 		     struct old_timeval32 __user *u_tv))
 {
-	return __cobalt_select(nfds, u_rfds, u_wfds, u_xfds, u_tv, true);
+	struct timespec64 ts64, *to = NULL;
+	struct __kernel_old_timeval tv;
+	int ret;
+
+	if (u_tv &&
+	    (!access_ok(u_tv, sizeof(tv)) || sys32_get_timeval(&tv, u_tv)))
+		return -EFAULT;
+
+	if (u_tv) {
+		ts64 = cobalt_timeval_to_timespec64(&tv);
+		to = &ts64;
+	}
+
+	ret = __cobalt_select(nfds, u_rfds, u_wfds, u_xfds, to, true);
+
+	if (u_tv) {
+		tv = cobalt_timespec64_to_timeval(to);
+		/* See CoBaLt_select() */
+		sys32_put_timeval(u_tv, &tv);
+	}
+
+	return ret;
 }
 
 COBALT_SYSCALL32emu(recvmsg, handover,

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 2/4] y2038: cobalt/posix/select: Adding pselect64
  2023-05-30 18:57 [PATCH v3 0/4] y2038: Part three - support for select() Florian Bezdeka
  2023-05-30 18:57 ` [PATCH v3 1/4] y2038: cobalt/posix/select: Refactor __cobalt_select() Florian Bezdeka
@ 2023-05-30 18:57 ` Florian Bezdeka
  2023-05-30 18:57 ` [PATCH v3 3/4] y2038: testsuite/smokey/y2038: Adding tests for pselect64 Florian Bezdeka
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Florian Bezdeka @ 2023-05-30 18:57 UTC (permalink / raw)
  To: jan.kiszka, xenomai; +Cc: Florian Bezdeka

This is not a full implementation of pselect() as we do not support
waiting for signals yet. The xnsynch infrastructure is not prepared
for that.

We will not wrap pselect() in userspace. We will only use the new
syscall for providing y2038 safety for select().

Another reason against a full pselec64 implementation is the fact that
the XENOMAI_SYSCALLx() infrastructure ends at XENOMAI_SYSCALL5(),
a real pselect64() implementation would require XENOMAI_SYSCALL6().

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 include/cobalt/uapi/syscall.h      |  1 +
 kernel/cobalt/posix/io.c           | 32 ++++++++++++++++++++++++++++++++
 kernel/cobalt/posix/io.h           |  8 +++++++-
 kernel/cobalt/trace/cobalt-posix.h |  3 ++-
 4 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/include/cobalt/uapi/syscall.h b/include/cobalt/uapi/syscall.h
index 2dca90071..3e65efaab 100644
--- a/include/cobalt/uapi/syscall.h
+++ b/include/cobalt/uapi/syscall.h
@@ -140,6 +140,7 @@
 #define sc_cobalt_timer_gettime64		117
 #define sc_cobalt_timerfd_settime64		118
 #define sc_cobalt_timerfd_gettime64		119
+#define sc_cobalt_pselect64			120
 
 #define __NR_COBALT_SYSCALLS			128 /* Power of 2 */
 
diff --git a/kernel/cobalt/posix/io.c b/kernel/cobalt/posix/io.c
index 786cb8ea9..f8e1b91c3 100644
--- a/kernel/cobalt/posix/io.c
+++ b/kernel/cobalt/posix/io.c
@@ -393,3 +393,35 @@ COBALT_SYSCALL(select, primary,
 
 	return ret;
 }
+
+COBALT_SYSCALL(pselect64, primary,
+	       (int nfds,
+		fd_set __user *u_rfds,
+		fd_set __user *u_wfds,
+		fd_set __user *u_xfds,
+		struct __kernel_timespec __user *u_ts))
+{
+	struct timespec64 ts64, *to = NULL;
+	int ret = 0;
+
+	if (u_ts) {
+		ret = cobalt_get_timespec64(&ts64, u_ts);
+		to = &ts64;
+	}
+
+	if (ret)
+		return ret;
+
+	ret = __cobalt_select(nfds, u_rfds, u_wfds, u_xfds, to, false);
+
+	/*
+	 * Normally pselect() would not write back the modified timeout. As we
+	 * only use it for keeping select() y2038 safe we do it here as well.
+	 *
+	 * See CoBaLt_select() for some notes about error handling.
+	 */
+	if (u_ts)
+		cobalt_put_timespec64(&ts64, u_ts);
+
+	return ret;
+}
diff --git a/kernel/cobalt/posix/io.h b/kernel/cobalt/posix/io.h
index 1d9ee0918..ca46aa309 100644
--- a/kernel/cobalt/posix/io.h
+++ b/kernel/cobalt/posix/io.h
@@ -24,7 +24,7 @@
 #include <cobalt/kernel/select.h>
 
 int __cobalt_select(int nfds, void __user *u_rfds, void __user *u_wfds,
-		    void __user *u_xfds, void __user *u_tv, bool compat);
+		    void __user *u_xfds, struct timespec64 *to, bool compat);
 
 COBALT_SYSCALL_DECL(open,
 		    (const char __user *u_path, int oflag));
@@ -76,4 +76,10 @@ COBALT_SYSCALL_DECL(select,
 		     fd_set __user *u_xfds,
 		     struct __kernel_old_timeval __user *u_tv));
 
+COBALT_SYSCALL_DECL(pselect64, (int nfds,
+				fd_set __user *u_rfds,
+				fd_set __user *u_wfds,
+				fd_set __user *u_xfds,
+				struct __kernel_timespec __user *u_tv));
+
 #endif /* !_COBALT_POSIX_IO_H */
diff --git a/kernel/cobalt/trace/cobalt-posix.h b/kernel/cobalt/trace/cobalt-posix.h
index f2ffdb3a2..47dc77e1c 100644
--- a/kernel/cobalt/trace/cobalt-posix.h
+++ b/kernel/cobalt/trace/cobalt-posix.h
@@ -172,7 +172,8 @@
 		__cobalt_symbolic_syscall(timer_settime64),		\
 		__cobalt_symbolic_syscall(timer_gettime64),		\
 		__cobalt_symbolic_syscall(timerfd_settime64),		\
-		__cobalt_symbolic_syscall(timerfd_gettime64))
+		__cobalt_symbolic_syscall(timerfd_gettime64),		\
+		__cobalt_symbolic_syscall(pselect64))
 
 DECLARE_EVENT_CLASS(cobalt_syscall_entry,
 	TP_PROTO(unsigned int nr),

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 3/4] y2038: testsuite/smokey/y2038: Adding tests for pselect64
  2023-05-30 18:57 [PATCH v3 0/4] y2038: Part three - support for select() Florian Bezdeka
  2023-05-30 18:57 ` [PATCH v3 1/4] y2038: cobalt/posix/select: Refactor __cobalt_select() Florian Bezdeka
  2023-05-30 18:57 ` [PATCH v3 2/4] y2038: cobalt/posix/select: Adding pselect64 Florian Bezdeka
@ 2023-05-30 18:57 ` Florian Bezdeka
  2023-05-30 18:57 ` [PATCH v3 4/4] cobalt/posix/select: Fix timeout update in case of -EINTR Florian Bezdeka
  2023-05-31  5:23 ` [PATCH v3 0/4] y2038: Part three - support for select() Jan Kiszka
  4 siblings, 0 replies; 6+ messages in thread
From: Florian Bezdeka @ 2023-05-30 18:57 UTC (permalink / raw)
  To: jan.kiszka, xenomai; +Cc: Florian Bezdeka

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

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

diff --git a/testsuite/smokey/y2038/syscall-tests.c b/testsuite/smokey/y2038/syscall-tests.c
index 2e3b81e0c..d1e1f46bf 100644
--- a/testsuite/smokey/y2038/syscall-tests.c
+++ b/testsuite/smokey/y2038/syscall-tests.c
@@ -1443,6 +1443,72 @@ out:
 	return ret;
 }
 
+static int test_sc_cobalt_pselect64(void)
+{
+	long sc_nr = sc_cobalt_pselect64;
+	struct xn_timespec64 t1, t2;
+	struct timespec ts_nat;
+	int ret;
+
+	/* Supplying an invalid timeout should deliver -EINVAL */
+	t1.tv_sec = -1;
+	t1.tv_nsec = 0;
+	ret = XENOMAI_SYSCALL5(sc_nr, NULL, NULL, NULL, NULL, &t1);
+	if (ret == -ENOSYS) {
+		smokey_note("cobalt_pselect64: skipped. (no kernel support)");
+		ret = 0;
+		goto out; // Not implemented, nothing to test, success
+	}
+	if (!smokey_assert(ret == -EINVAL)) {
+		ret = ret ?: -EINVAL;
+		goto out;
+	}
+
+	/* Supplying an invalid address should deliver -EFAULT */
+	ret = XENOMAI_SYSCALL5(sc_nr, NULL, NULL, NULL, NULL,
+			       (void *)0xdeadbeefUL);
+	if (!smokey_assert(ret == -EFAULT)) {
+		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;
+
+	ret = XENOMAI_SYSCALL5(sc_nr, NULL, NULL, NULL, NULL, &t1);
+	if (!smokey_assert(!ret)) {
+		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("pselect64 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);
+
+out:
+	return ret;
+}
+
 static int check_kernel_version(void)
 {
 	int ret, major, minor;
@@ -1548,5 +1614,9 @@ static int run_y2038(struct smokey_test *t, int argc, char *const argv[])
 	if (ret)
 		return ret;
 
+	ret = test_sc_cobalt_pselect64();
+	if (ret)
+		return ret;
+
 	return 0;
 }

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 4/4] cobalt/posix/select: Fix timeout update in case of -EINTR
  2023-05-30 18:57 [PATCH v3 0/4] y2038: Part three - support for select() Florian Bezdeka
                   ` (2 preceding siblings ...)
  2023-05-30 18:57 ` [PATCH v3 3/4] y2038: testsuite/smokey/y2038: Adding tests for pselect64 Florian Bezdeka
@ 2023-05-30 18:57 ` Florian Bezdeka
  2023-05-31  5:23 ` [PATCH v3 0/4] y2038: Part three - support for select() Jan Kiszka
  4 siblings, 0 replies; 6+ messages in thread
From: Florian Bezdeka @ 2023-05-30 18:57 UTC (permalink / raw)
  To: jan.kiszka, xenomai; +Cc: Florian Bezdeka

When the select() syscall was interrupted (e.g. for handling a Linux
signal) the supplied timeout has not been updated. __cobalt_select()
bailed out too early.

This is not a critical thing as the manpage states:
  On  Linux,  select()  modifies timeout to reflect the amount of time
  not slept; most other implementations do not do this.  (POSIX.1
  permits either behavior.)

  This causes problems both when Linux code
  which reads timeout is ported to other operating systems, and when
  code is ported to Linux that reuses a struct timeval for multiple
  select()s in a loop without reinitializing it.  Consider timeout to
  be undefined after select() returns."

The Xenomai select() implementation is now aligned with Linux.

The testsuite has been improved to cover this case as well.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 kernel/cobalt/posix/io.c               | 18 +++----
 testsuite/smokey/y2038/syscall-tests.c | 94 +++++++++++++++++++++++++++++++++-
 2 files changed, 102 insertions(+), 10 deletions(-)

diff --git a/kernel/cobalt/posix/io.c b/kernel/cobalt/posix/io.c
index f8e1b91c3..9e6f3edfa 100644
--- a/kernel/cobalt/posix/io.c
+++ b/kernel/cobalt/posix/io.c
@@ -319,6 +319,15 @@ int __cobalt_select(int nfds, void __user *u_rfds, void __user *u_wfds,
 		}
 	} while (err == -ECHRNG);
 
+out:
+	if (to && (err > 0 || err == -EINTR)) {
+		xnsticks_t diff = timeout - clock_get_ticks(CLOCK_MONOTONIC);
+		if (diff > 0)
+			ticks2ts64(to, diff);
+		else
+			to->tv_sec = to->tv_nsec = 0;
+	}
+
 	if (err == -EINTR && signal_pending(current)) {
 		xnthread_set_localinfo(curr, XNSYSRST);
 
@@ -329,15 +338,6 @@ int __cobalt_select(int nfds, void __user *u_rfds, void __user *u_wfds,
 		return -ERESTARTSYS;
 	}
 
-out:
-	if (to && (err > 0 || err == -EINTR)) {
-		xnsticks_t diff = timeout - clock_get_ticks(CLOCK_MONOTONIC);
-		if (diff > 0)
-			ticks2ts64(to, diff);
-		else
-			to->tv_sec = to->tv_nsec = 0;
-	}
-
 	if (err >= 0)
 		for (i = 0; i < XNSELECT_MAX_TYPES; i++) {
 			if (!ufd_sets[i])
diff --git a/testsuite/smokey/y2038/syscall-tests.c b/testsuite/smokey/y2038/syscall-tests.c
index d1e1f46bf..af27f2bce 100644
--- a/testsuite/smokey/y2038/syscall-tests.c
+++ b/testsuite/smokey/y2038/syscall-tests.c
@@ -123,6 +123,7 @@ static inline bool ts_less(const struct xn_timespec64 *a,
  */
 struct thread_context {
 	int sc_nr;
+	int sock;
 	pthread_mutex_t *mutex;
 	struct xn_timespec64 *ts;
 	bool timedwait_timecheck;
@@ -1443,6 +1444,94 @@ out:
 	return ret;
 }
 
+static void pselect64_sig_handler(int sig)
+{
+	smokey_trace("pselect64: signal received");
+}
+
+static void *pselect64_waiting_thread(void *arg)
+{
+	struct thread_context *ctx = (struct thread_context *)arg;
+	struct xn_timespec64 ts;
+	int sock = ctx->sock;
+	struct sigaction sa;
+	fd_set rfds;
+	int ret;
+
+	FD_ZERO(&rfds);
+	FD_SET(ctx->sock, &rfds);
+
+	sigemptyset(&sa.sa_mask);
+	sa.sa_flags = SA_SIGINFO;
+	sa.sa_handler = pselect64_sig_handler;
+
+	ret = smokey_check_errno(sigaction(SIGUSR1, &sa, NULL));
+	if (ret)
+		goto out;
+
+	/* Waiting for 1sec should be enough to get a signal */
+	ts.tv_sec = 1;
+	ts.tv_nsec = 0;
+	ret = XENOMAI_SYSCALL5(ctx->sc_nr, sock + 1, &rfds, NULL, NULL, &ts);
+	if (!smokey_assert(ret == -EINTR)) {
+		ret = ret ?: -EINVAL;
+		goto out;
+	}
+
+	/* The remaining timeout has to be less than 1 sec */
+	if (!smokey_assert(ts.tv_sec == 0 && ts.tv_nsec > 0)) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+out:
+	return (void *)(long)ret;
+}
+
+static int test_pselect64_interruption(void)
+{
+	struct timespec ts = { .tv_sec = 0, .tv_nsec = 100000 };
+	struct thread_context ctx = { 0 };
+	void *w_ret = NULL;
+	pthread_t w;
+	int sock;
+	int ret;
+
+	sock = smokey_check_errno(socket(AF_RTIPC, SOCK_DGRAM, IPCPROTO_XDDP));
+	if (sock == -EAFNOSUPPORT) {
+		smokey_note("pselect64: skipped timeout write back test");
+		return 0;
+	}
+	if (sock < 0)
+		return sock;
+
+	ctx.sock = sock;
+	ctx.sc_nr = sc_cobalt_pselect64;
+
+	ret = smokey_check_status(
+		pthread_create(&w, NULL, pselect64_waiting_thread, &ctx));
+	if (ret)
+		goto out_sock;
+
+	/* Allow the waiting thread to enter the pselect64() syscall */
+	nanosleep(&ts, NULL);
+
+	/* Send a signal to interrupt the syscall and trigger -EINTR */
+	__STD(pthread_kill(w, SIGUSR1));
+
+	ret = smokey_check_status(pthread_join(w, &w_ret));
+	if (ret)
+		goto out_sock;
+
+	ret = smokey_assert((long)w_ret == -EINTR);
+	if (ret)
+		w_ret = NULL;
+
+out_sock:
+	close(sock);
+	return (int)(long)w_ret;
+}
+
 static int test_sc_cobalt_pselect64(void)
 {
 	long sc_nr = sc_cobalt_pselect64;
@@ -1506,7 +1595,10 @@ static int test_sc_cobalt_pselect64(void)
 			       t1.tv_sec, t1.tv_nsec, t2.tv_sec, t2.tv_nsec);
 
 out:
-	return ret;
+	if (ret)
+		return ret;
+
+	return test_pselect64_interruption();
 }
 
 static int check_kernel_version(void)

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 0/4] y2038: Part three - support for select()
  2023-05-30 18:57 [PATCH v3 0/4] y2038: Part three - support for select() Florian Bezdeka
                   ` (3 preceding siblings ...)
  2023-05-30 18:57 ` [PATCH v3 4/4] cobalt/posix/select: Fix timeout update in case of -EINTR Florian Bezdeka
@ 2023-05-31  5:23 ` Jan Kiszka
  4 siblings, 0 replies; 6+ messages in thread
From: Jan Kiszka @ 2023-05-31  5:23 UTC (permalink / raw)
  To: Florian Bezdeka, xenomai

On 30.05.23 20:57, Florian Bezdeka wrote:
> Hi all,
> 
> I'm trying to bring the remaining patches from my y2038 queue into Xenomai 
> next/master branches. The full queue [1] holds ~7 patches. I'm trying to 
> split that up to keep reviewing efforts low.
> 
> This series brings y2038 support for select().
> 
> select() is somehow special. Linux does not support a y2038 safe
> implementation for select() itself. If the application needs y2038
> support the application has to use something like the pselect() service.
> 
> To make select() y2038 safe glibc wrapps select() and wires it to the 
> pselect() Linux service.
> 
> Xenomai (cobalt) does not provide pselect() yet and it's not easy to 
> implement that. The reasons:
> 
>   - The underlying xnsynch infrastructure is not prepared for waiting for a
>     combination of file descriptors and signals. (Let me know if I
>     overlooked something!)
> 
>   - Implementing pselect() would require up to 6 parameters that need to
>     be forwarded to the kernel. Xenomais current limit is 5.
> 
> To get select() y2038 safe a "pselect64() like" service service is being
> added. The signal part is skipped.
> 
> The wrapper for __select64() will be added later and will convert 
> struct timeval to struct timespec64 in user space. This way select() 
> provided by libcobalt gets y2038 support but pselect() is still not 
> supported.
> 
> Best regards,
> Florian
> 
> [1] https://gitlab.com/Xenomai/xenomai-hacker-space/-/tree/florian/y2038
> 
> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> ---
> Changes in v3:
> - Patch 1: Remove trailing comments, document error handling 
> - Patch 2: Remove trailing comment, document error handling
> - Patch 4: Rewrite commit message
> - Link to v2: https://lore.kernel.org/r/20230516-florian-y2038-part-three-v2-0-3fc689b21a32@siemens.com
> 
> Changes in v2:
> - Patch 1: Do not skip updating the timeout if fds changed
>            Fix is in all entry points.
> - Patch 2: pselect64: Same as above. Do not skip the update if fds
>            changed.
> - Patch 3: unmodified
> - Patch 4: New patch. Fix timeout update in case of -EINTR
> 
> - Link to v1: https://lore.kernel.org/r/20230516-florian-y2038-part-three-v1-0-b140278b26c6@siemens.com
> 
> ---
> Florian Bezdeka (4):
>       y2038: cobalt/posix/select: Refactor __cobalt_select()
>       y2038: cobalt/posix/select: Adding pselect64
>       y2038: testsuite/smokey/y2038: Adding tests for pselect64
>       cobalt/posix/select: Fix timeout update in case of -EINTR
> 
>  include/cobalt/uapi/syscall.h                      |   1 +
>  .../cobalt/include/asm-generic/xenomai/syscall.h   |  22 +++
>  kernel/cobalt/posix/clock.h                        |   6 +-
>  kernel/cobalt/posix/io.c                           | 109 +++++++++-----
>  kernel/cobalt/posix/io.h                           |   8 +-
>  kernel/cobalt/posix/syscall32.c                    |  23 ++-
>  kernel/cobalt/trace/cobalt-posix.h                 |   3 +-
>  testsuite/smokey/y2038/syscall-tests.c             | 162 +++++++++++++++++++++
>  8 files changed, 290 insertions(+), 44 deletions(-)
> ---
> base-commit: a74abe52269f191654b449357bd9aaa57f2cd75c
> change-id: 20230516-florian-y2038-part-three-3093da9fecf8
> 
> Best regards,

Thanks, applied.

Jan
-- 
Siemens AG, Technology
Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-05-31  5:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-30 18:57 [PATCH v3 0/4] y2038: Part three - support for select() Florian Bezdeka
2023-05-30 18:57 ` [PATCH v3 1/4] y2038: cobalt/posix/select: Refactor __cobalt_select() Florian Bezdeka
2023-05-30 18:57 ` [PATCH v3 2/4] y2038: cobalt/posix/select: Adding pselect64 Florian Bezdeka
2023-05-30 18:57 ` [PATCH v3 3/4] y2038: testsuite/smokey/y2038: Adding tests for pselect64 Florian Bezdeka
2023-05-30 18:57 ` [PATCH v3 4/4] cobalt/posix/select: Fix timeout update in case of -EINTR Florian Bezdeka
2023-05-31  5:23 ` [PATCH v3 0/4] y2038: Part three - support for select() Jan Kiszka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).