On systems using 32 bit for time_t the sem_timedwait syscall was broken
because the function used for copying the timeout value from userspace
to kernel (=sem_fetch_timeout()) was always copying
sizeof(struct timespec64).
A 32 bit application (or more specific an application with 4 byte
time_t) would only provide sizeof(struct old_timespec32).
Notable changes:
- The copy operation from userspace to kernel is now already done in
the syscall handler. So it is always done. Previously it was copied
over and validated before the first use (when used at all).
So we have some additional instructions now that may be
unnecessary, but that simplifies the code.
- Validation: Switched to timespec64_valid() instead of our own
check.
Fixes: 8043eccd232d ("cobalt/kernel: y2038: convert struct timespec to
timespec64")
Signed-off-by: Florian Bezdeka <[email protected]>
---
kernel/cobalt/posix/sem.c | 40 +++++++++++++++------------------
kernel/cobalt/posix/sem.h | 6 ++---
kernel/cobalt/posix/syscall32.c | 10 +++++++--
kernel/cobalt/posix/syscall32.h | 2 +-
4 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/kernel/cobalt/posix/sem.c b/kernel/cobalt/posix/sem.c
index 467a9b7dd..827a4751a 100644
--- a/kernel/cobalt/posix/sem.c
+++ b/kernel/cobalt/posix/sem.c
@@ -267,20 +267,11 @@ out:
return ret;
}
-static inline int sem_fetch_timeout(struct timespec64 *ts,
- const void __user *u_ts)
-{
- return u_ts == NULL ? -EFAULT :
- cobalt_copy_from_user(ts, u_ts, sizeof(*ts));
-}
-
int __cobalt_sem_timedwait(struct cobalt_sem_shadow __user *u_sem,
- const void __user *u_ts,
- int (*fetch_timeout)(struct timespec64 *ts,
- const void __user *u_ts))
+ const struct timespec64 *ts)
{
- struct timespec64 ts = { .tv_sec = 0, .tv_nsec = 0 };
- int pull_ts = 1, ret, info;
+ int ret, info;
+ bool validate_ts = true;
struct cobalt_sem *sem;
xnhandle_t handle;
xntmode_t tmode;
@@ -304,24 +295,23 @@ int __cobalt_sem_timedwait(struct cobalt_sem_shadow
__user *u_sem,
* it's actually more complex, to keep some
* applications ported to Linux happy.
*/
- if (pull_ts) {
+ if (validate_ts) {
atomic_inc(&sem->state->value);
- xnlock_put_irqrestore(&nklock, s);
- ret = fetch_timeout(&ts, u_ts);
- xnlock_get_irqsave(&nklock, s);
- if (ret)
+ if (!ts) {
+ ret = -EFAULT;
break;
- if (ts.tv_nsec >= ONE_BILLION) {
+ }
+ if (!timespec64_valid(ts)) {
ret = -EINVAL;
break;
}
- pull_ts = 0;
+ validate_ts = false;
continue;
}
ret = 0;
tmode = sem->flags & SEM_RAWCLOCK ? XN_ABSOLUTE : XN_REALTIME;
- info = xnsynch_sleep_on(&sem->synchbase, ts2ns(&ts) + 1, tmode);
+ info = xnsynch_sleep_on(&sem->synchbase, ts2ns(ts) + 1, tmode);
if (info & XNRMID)
ret = -EINVAL;
else if (info & (XNBREAK|XNTIMEO)) {
@@ -434,9 +424,15 @@ COBALT_SYSCALL(sem_wait, primary,
COBALT_SYSCALL(sem_timedwait, primary,
(struct cobalt_sem_shadow __user *u_sem,
- struct __user_old_timespec __user *u_ts))
+ const struct __user_old_timespec __user *u_ts))
{
- return __cobalt_sem_timedwait(u_sem, u_ts, sem_fetch_timeout);
+ int ret = 1;
+ struct timespec64 ts64;
+
+ if (u_ts)
+ ret = cobalt_get_u_timespec(&ts64, u_ts);
+
+ return __cobalt_sem_timedwait(u_sem, ret ? NULL : &ts64);
}
COBALT_SYSCALL(sem_trywait, primary,
diff --git a/kernel/cobalt/posix/sem.h b/kernel/cobalt/posix/sem.h
index d17299495..658e11f7a 100644
--- a/kernel/cobalt/posix/sem.h
+++ b/kernel/cobalt/posix/sem.h
@@ -64,9 +64,7 @@ __cobalt_sem_open(struct cobalt_sem_shadow __user *usm,
int oflags, mode_t mode, unsigned int value);
int __cobalt_sem_timedwait(struct cobalt_sem_shadow __user *u_sem,
- const void __user *u_ts,
- int (*fetch_timeout)(struct timespec64 *ts,
- const void __user *u_ts));
+ const struct timespec64 *ts);
int __cobalt_sem_destroy(xnhandle_t handle);
@@ -91,7 +89,7 @@ COBALT_SYSCALL_DECL(sem_wait,
COBALT_SYSCALL_DECL(sem_timedwait,
(struct cobalt_sem_shadow __user *u_sem,
- struct __user_old_timespec __user *u_ts));
+ const struct __user_old_timespec __user *u_ts));
COBALT_SYSCALL_DECL(sem_trywait,
(struct cobalt_sem_shadow __user *u_sem));
diff --git a/kernel/cobalt/posix/syscall32.c b/kernel/cobalt/posix/syscall32.c
index 57aa7251a..edac7ea4a 100644
--- a/kernel/cobalt/posix/syscall32.c
+++ b/kernel/cobalt/posix/syscall32.c
@@ -124,9 +124,15 @@ COBALT_SYSCALL32emu(sem_open, lostage,
COBALT_SYSCALL32emu(sem_timedwait, primary,
(struct cobalt_sem_shadow __user *u_sem,
- struct old_timespec32 __user *u_ts))
+ const struct old_timespec32 __user *u_ts))
{
- return __cobalt_sem_timedwait(u_sem, u_ts, sys32_fetch_timeout);
+ int ret = 1;
+ struct timespec64 ts64;
+
+ if (u_ts)
+ ret = sys32_fetch_timeout(&ts64, u_ts);
+
+ return __cobalt_sem_timedwait(u_sem, ret ? NULL : &ts64);
}
COBALT_SYSCALL32emu(clock_getres, current,
diff --git a/kernel/cobalt/posix/syscall32.h b/kernel/cobalt/posix/syscall32.h
index 66cd2a5d2..d72fd2022 100644
--- a/kernel/cobalt/posix/syscall32.h
+++ b/kernel/cobalt/posix/syscall32.h
@@ -229,6 +229,6 @@ COBALT_SYSCALL32emu_DECL(sem_open,
COBALT_SYSCALL32emu_DECL(sem_timedwait,
(struct cobalt_sem_shadow __user *u_sem,
- struct old_timespec32 __user *u_ts));
+ const struct old_timespec32 __user *u_ts));
#endif /* !_COBALT_POSIX_SYSCALL32_H */
--
2.29.2
