All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] alpha: osf_sys.c: fix put_tv32 regression
@ 2017-11-07 14:09 Arnd Bergmann
  2017-11-07 14:09 ` [PATCH 2/2] alpha: osf_sys.c: use timespec64 where appropriate Arnd Bergmann
  2017-11-07 15:52 ` [PATCH 1/2] alpha: osf_sys.c: fix put_tv32 regression Al Viro
  0 siblings, 2 replies; 6+ messages in thread
From: Arnd Bergmann @ 2017-11-07 14:09 UTC (permalink / raw)
  To: Richard Henderson
  Cc: Ivan Kokshaysky, Matt Turner, Alexander Viro, y2038,
	Deepa Dinamani, Arnd Bergmann, stable, linux-alpha, linux-kernel

There was a typo in the new version of put_tv32() that caused
uninitialized stack data to be written back to user space, rather
than writing the actual timeval for the emulation of
gettimeofday(), wait4(), usleep_thread() and old_adjtimex().

This fixes it to write the correct data again.

Cc: stable@vger.kernel.org
Fixes: 1cc6c4635e9f ("osf_sys.c: switch handling of timeval32/itimerval32 to copy_{to,from}_user()")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/alpha/kernel/osf_sys.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c
index ce3a675c0c4b..75a5c35a2067 100644
--- a/arch/alpha/kernel/osf_sys.c
+++ b/arch/alpha/kernel/osf_sys.c
@@ -964,8 +964,8 @@ static inline long
 put_tv32(struct timeval32 __user *o, struct timeval *i)
 {
 	return copy_to_user(o, &(struct timeval32){
-				.tv_sec = o->tv_sec,
-				.tv_usec = o->tv_usec},
+				.tv_sec = i->tv_sec,
+				.tv_usec = i->tv_usec},
 			    sizeof(struct timeval32));
 }
 
-- 
2.9.0

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

* [PATCH 2/2] alpha: osf_sys.c: use timespec64 where appropriate
  2017-11-07 14:09 [PATCH 1/2] alpha: osf_sys.c: fix put_tv32 regression Arnd Bergmann
@ 2017-11-07 14:09 ` Arnd Bergmann
  2017-11-08  0:22   ` [Y2038] " Ben Hutchings
  2017-11-07 15:52 ` [PATCH 1/2] alpha: osf_sys.c: fix put_tv32 regression Al Viro
  1 sibling, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2017-11-07 14:09 UTC (permalink / raw)
  To: Richard Henderson
  Cc: Ivan Kokshaysky, Matt Turner, Alexander Viro, y2038,
	Deepa Dinamani, Arnd Bergmann, linux-alpha, linux-kernel

Some of the syscall helper functions (do_utimes, poll_select_set_timeout,
core_sys_select) have changed over the past year or two to use
'timespec64' pointers rather than 'timespec'. This was fine on alpha,
since 64-bit architectures treat the two as the same type.

However, I'd like to change that behavior and make 'timespec64' a proper
type of its own even on 64-bit architectures, and that will introduce
harmless type mismatch warnings here.

Also, I'm trying to kill off the do_gettimeofday() helper in favor of
ktime_get() and related interfaces throughout the kernel.

This changes the get_tv32/put_tv32 helper functions to also take a
timespec64 argument rather than timeval, which allows us to simplify
some of the syscall helpers a bit and avoid the type warnings.

For the moment, wait4 and adjtimex are still better off with the old
behavior, so I'm adding a special put_tv_to_tv32() helper for those.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/alpha/kernel/osf_sys.c | 68 ++++++++++++++++++++++-----------------------
 1 file changed, 34 insertions(+), 34 deletions(-)

diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c
index 75a5c35a2067..6db6ec30c3de 100644
--- a/arch/alpha/kernel/osf_sys.c
+++ b/arch/alpha/kernel/osf_sys.c
@@ -950,18 +950,27 @@ struct itimerval32
 };
 
 static inline long
-get_tv32(struct timeval *o, struct timeval32 __user *i)
+get_tv32(struct timespec64 *o, struct timeval32 __user *i)
 {
 	struct timeval32 tv;
 	if (copy_from_user(&tv, i, sizeof(struct timeval32)))
 		return -EFAULT;
 	o->tv_sec = tv.tv_sec;
-	o->tv_usec = tv.tv_usec;
+	o->tv_nsec = tv.tv_usec * NSEC_PER_USEC;
 	return 0;
 }
 
 static inline long
-put_tv32(struct timeval32 __user *o, struct timeval *i)
+put_tv32(struct timeval32 __user *o, struct timespec64 *i)
+{
+	return copy_to_user(o, &(struct timeval32){
+				.tv_sec = i->tv_sec,
+				.tv_usec = i->tv_nsec / NSEC_PER_USEC},
+			    sizeof(struct timeval32));
+}
+
+static inline long
+put_tv_to_tv32(struct timeval32 __user *o, struct timeval *i)
 {
 	return copy_to_user(o, &(struct timeval32){
 				.tv_sec = i->tv_sec,
@@ -1004,9 +1013,10 @@ SYSCALL_DEFINE2(osf_gettimeofday, struct timeval32 __user *, tv,
 		struct timezone __user *, tz)
 {
 	if (tv) {
-		struct timeval ktv;
-		do_gettimeofday(&ktv);
-		if (put_tv32(tv, &ktv))
+		struct timespec64 kts;
+
+		ktime_get_ts64(&kts);
+		if (put_tv32(tv, &kts))
 			return -EFAULT;
 	}
 	if (tz) {
@@ -1019,22 +1029,19 @@ SYSCALL_DEFINE2(osf_gettimeofday, struct timeval32 __user *, tv,
 SYSCALL_DEFINE2(osf_settimeofday, struct timeval32 __user *, tv,
 		struct timezone __user *, tz)
 {
-	struct timespec64 kts64;
-	struct timespec kts;
+	struct timespec64 kts;
 	struct timezone ktz;
 
  	if (tv) {
-		if (get_tv32((struct timeval *)&kts, tv))
+		if (get_tv32(&kts, tv))
 			return -EFAULT;
-		kts.tv_nsec *= 1000;
-		kts64 = timespec_to_timespec64(kts);
 	}
 	if (tz) {
 		if (copy_from_user(&ktz, tz, sizeof(*tz)))
 			return -EFAULT;
 	}
 
-	return do_sys_settimeofday64(tv ? &kts64 : NULL, tz ? &ktz : NULL);
+	return do_sys_settimeofday64(tv ? &kts : NULL, tz ? &ktz : NULL);
 }
 
 asmlinkage long sys_ni_posix_timers(void);
@@ -1083,22 +1090,16 @@ SYSCALL_DEFINE3(osf_setitimer, int, which, struct itimerval32 __user *, in,
 SYSCALL_DEFINE2(osf_utimes, const char __user *, filename,
 		struct timeval32 __user *, tvs)
 {
-	struct timespec tv[2];
+	struct timespec64 tv[2];
 
 	if (tvs) {
-		struct timeval ktvs[2];
-		if (get_tv32(&ktvs[0], &tvs[0]) ||
-		    get_tv32(&ktvs[1], &tvs[1]))
+		if (get_tv32(&tv[0], &tvs[0]) ||
+		    get_tv32(&tv[1], &tvs[1]))
 			return -EFAULT;
 
-		if (ktvs[0].tv_usec < 0 || ktvs[0].tv_usec >= 1000000 ||
-		    ktvs[1].tv_usec < 0 || ktvs[1].tv_usec >= 1000000)
+		if (tv[0].tv_nsec < 0 || tv[0].tv_nsec >= 1000000000 ||
+		    tv[1].tv_nsec < 0 || tv[1].tv_nsec >= 1000000000)
 			return -EINVAL;
-
-		tv[0].tv_sec = ktvs[0].tv_sec;
-		tv[0].tv_nsec = 1000 * ktvs[0].tv_usec;
-		tv[1].tv_sec = ktvs[1].tv_sec;
-		tv[1].tv_nsec = 1000 * ktvs[1].tv_usec;
 	}
 
 	return do_utimes(AT_FDCWD, filename, tvs ? tv : NULL, 0);
@@ -1107,19 +1108,18 @@ SYSCALL_DEFINE2(osf_utimes, const char __user *, filename,
 SYSCALL_DEFINE5(osf_select, int, n, fd_set __user *, inp, fd_set __user *, outp,
 		fd_set __user *, exp, struct timeval32 __user *, tvp)
 {
-	struct timespec end_time, *to = NULL;
+	struct timespec64 end_time, *to = NULL;
 	if (tvp) {
-		struct timeval tv;
+		struct timespec64 tv;
 		to = &end_time;
 
 		if (get_tv32(&tv, tvp))
 		    	return -EFAULT;
 
-		if (tv.tv_sec < 0 || tv.tv_usec < 0)
+		if (tv.tv_sec < 0 || tv.tv_nsec < 0)
 			return -EINVAL;
 
-		if (poll_select_set_timeout(to, tv.tv_sec,
-					    tv.tv_usec * NSEC_PER_USEC))
+		if (poll_select_set_timeout(to, tv.tv_sec, tv.tv_nsec))
 			return -EINVAL;		
 
 	}
@@ -1192,9 +1192,9 @@ SYSCALL_DEFINE4(osf_wait4, pid_t, pid, int __user *, ustatus, int, options,
 		return -EFAULT;
 	if (!ur)
 		return err;
-	if (put_tv32(&ur->ru_utime, &r.ru_utime))
+	if (put_tv_to_tv32(&ur->ru_utime, &r.ru_utime))
 		return -EFAULT;
-	if (put_tv32(&ur->ru_stime, &r.ru_stime))
+	if (put_tv_to_tv32(&ur->ru_stime, &r.ru_stime))
 		return -EFAULT;
 	if (copy_to_user(&ur->ru_maxrss, &r.ru_maxrss,
 	      sizeof(struct rusage32) - offsetof(struct rusage32, ru_maxrss)))
@@ -1210,18 +1210,18 @@ SYSCALL_DEFINE4(osf_wait4, pid_t, pid, int __user *, ustatus, int, options,
 SYSCALL_DEFINE2(osf_usleep_thread, struct timeval32 __user *, sleep,
 		struct timeval32 __user *, remain)
 {
-	struct timeval tmp;
+	struct timespec64 tmp;
 	unsigned long ticks;
 
 	if (get_tv32(&tmp, sleep))
 		goto fault;
 
-	ticks = timeval_to_jiffies(&tmp);
+	ticks = timespec64_to_jiffies(&tmp);
 
 	ticks = schedule_timeout_interruptible(ticks);
 
 	if (remain) {
-		jiffies_to_timeval(ticks, &tmp);
+		jiffies_to_timespec64(ticks, &tmp);
 		if (put_tv32(remain, &tmp))
 			goto fault;
 	}
@@ -1280,7 +1280,7 @@ SYSCALL_DEFINE1(old_adjtimex, struct timex32 __user *, txc_p)
 	if (copy_to_user(txc_p, &txc, offsetof(struct timex32, time)) ||
 	    (copy_to_user(&txc_p->tick, &txc.tick, sizeof(struct timex32) - 
 			  offsetof(struct timex32, tick))) ||
-	    (put_tv32(&txc_p->time, &txc.time)))
+	    (put_tv_to_tv32(&txc_p->time, &txc.time)))
 	  return -EFAULT;
 
 	return ret;
-- 
2.9.0

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

* Re: [PATCH 1/2] alpha: osf_sys.c: fix put_tv32 regression
  2017-11-07 14:09 [PATCH 1/2] alpha: osf_sys.c: fix put_tv32 regression Arnd Bergmann
  2017-11-07 14:09 ` [PATCH 2/2] alpha: osf_sys.c: use timespec64 where appropriate Arnd Bergmann
@ 2017-11-07 15:52 ` Al Viro
  2017-11-07 16:03   ` Arnd Bergmann
  1 sibling, 1 reply; 6+ messages in thread
From: Al Viro @ 2017-11-07 15:52 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Richard Henderson, Ivan Kokshaysky, Matt Turner, y2038,
	Deepa Dinamani, stable, linux-alpha, linux-kernel

On Tue, Nov 07, 2017 at 03:09:24PM +0100, Arnd Bergmann wrote:
> There was a typo in the new version of put_tv32() that caused
> uninitialized stack data to be written back to user space, rather
> than writing the actual timeval for the emulation of
> gettimeofday(), wait4(), usleep_thread() and old_adjtimex().
> 
> This fixes it to write the correct data again.

*blink*

the bug is real, all right, and the fix is correct one, but where
do you get an infoleak?  What it is is a user-triggerable oops -
just pass it an unmapped address.  For anything mapped r/w it's
simply a no-op - userland data is unchanged.

IOW, the fix is correct, but commit message isn't - it's

"user-triggerable oops and in all cases failed to modify userland timeval32"

not

"uninitialized stack data to be written back to user space"

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

* Re: [PATCH 1/2] alpha: osf_sys.c: fix put_tv32 regression
  2017-11-07 15:52 ` [PATCH 1/2] alpha: osf_sys.c: fix put_tv32 regression Al Viro
@ 2017-11-07 16:03   ` Arnd Bergmann
  0 siblings, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2017-11-07 16:03 UTC (permalink / raw)
  To: Al Viro
  Cc: Richard Henderson, Ivan Kokshaysky, Matt Turner,
	y2038 Mailman List, Deepa Dinamani, # 3.4.x, linux-alpha,
	Linux Kernel Mailing List

On Tue, Nov 7, 2017 at 4:52 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Tue, Nov 07, 2017 at 03:09:24PM +0100, Arnd Bergmann wrote:
>> There was a typo in the new version of put_tv32() that caused
>> uninitialized stack data to be written back to user space, rather
>> than writing the actual timeval for the emulation of
>> gettimeofday(), wait4(), usleep_thread() and old_adjtimex().
>>
>> This fixes it to write the correct data again.
>
> *blink*
>
> the bug is real, all right, and the fix is correct one, but where
> do you get an infoleak?  What it is is a user-triggerable oops -
> just pass it an unmapped address.  For anything mapped r/w it's
> simply a no-op - userland data is unchanged.
>
> IOW, the fix is correct, but commit message isn't - it's
>
> "user-triggerable oops and in all cases failed to modify userland timeval32"
>
> not
>
> "uninitialized stack data to be written back to user space"

Ah right, sorry about that. I misread the statement as setting
the temporary structure to itself rather than setting it to the contents
of the user structure.

Do you want to update the description as suggested and forward it,
or should I send a fixed version? I'm about to leave the office for
today, so I'd have to do it tomorrow then.

       Arnd

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

* Re: [Y2038] [PATCH 2/2] alpha: osf_sys.c: use timespec64 where appropriate
  2017-11-07 14:09 ` [PATCH 2/2] alpha: osf_sys.c: use timespec64 where appropriate Arnd Bergmann
@ 2017-11-08  0:22   ` Ben Hutchings
  2017-11-08 14:54     ` Arnd Bergmann
  0 siblings, 1 reply; 6+ messages in thread
From: Ben Hutchings @ 2017-11-08  0:22 UTC (permalink / raw)
  To: Arnd Bergmann, Richard Henderson
  Cc: y2038, linux-kernel, Ivan Kokshaysky, Alexander Viro,
	linux-alpha, Matt Turner, Deepa Dinamani

On Tue, 2017-11-07 at 15:09 +0100, Arnd Bergmann wrote:
> Some of the syscall helper functions (do_utimes, poll_select_set_timeout,
> core_sys_select) have changed over the past year or two to use
> 'timespec64' pointers rather than 'timespec'. This was fine on alpha,
> since 64-bit architectures treat the two as the same type.
> 
> However, I'd like to change that behavior and make 'timespec64' a proper
> type of its own even on 64-bit architectures, and that will introduce
> harmless type mismatch warnings here.
> 
> Also, I'm trying to kill off the do_gettimeofday() helper in favor of
> ktime_get() and related interfaces throughout the kernel.
[...]
> @@ -1004,9 +1013,10 @@ SYSCALL_DEFINE2(osf_gettimeofday, struct timeval32 __user *, tv,
>  		struct timezone __user *, tz)
>  {
>  	if (tv) {
> -		struct timeval ktv;
> -		do_gettimeofday(&ktv);
> -		if (put_tv32(tv, &ktv))
> +		struct timespec64 kts;
> +
> +		ktime_get_ts64(&kts);
[...]

But this syscall is supposed to use the realtime clock, no?  It seems
like the correct substitute here is getnstimeofday64() (as the kernel-
doc comment for do_gettimeofday() *almost* says).

Ben.

-- 
Ben Hutchings
Software Developer, Codethink Ltd.

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

* Re: [Y2038] [PATCH 2/2] alpha: osf_sys.c: use timespec64 where appropriate
  2017-11-08  0:22   ` [Y2038] " Ben Hutchings
@ 2017-11-08 14:54     ` Arnd Bergmann
  0 siblings, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2017-11-08 14:54 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Richard Henderson, y2038 Mailman List, Linux Kernel Mailing List,
	Ivan Kokshaysky, Alexander Viro, linux-alpha, Matt Turner,
	Deepa Dinamani

On Wed, Nov 8, 2017 at 1:22 AM, Ben Hutchings
<ben.hutchings@codethink.co.uk> wrote:
> On Tue, 2017-11-07 at 15:09 +0100, Arnd Bergmann wrote:
>> Some of the syscall helper functions (do_utimes, poll_select_set_timeout,
>> core_sys_select) have changed over the past year or two to use
>> 'timespec64' pointers rather than 'timespec'. This was fine on alpha,
>> since 64-bit architectures treat the two as the same type.
>>
>> However, I'd like to change that behavior and make 'timespec64' a proper
>> type of its own even on 64-bit architectures, and that will introduce
>> harmless type mismatch warnings here.
>>
>> Also, I'm trying to kill off the do_gettimeofday() helper in favor of
>> ktime_get() and related interfaces throughout the kernel.
> [...]
>> @@ -1004,9 +1013,10 @@ SYSCALL_DEFINE2(osf_gettimeofday, struct timeval32 __user *, tv,
>>               struct timezone __user *, tz)
>>  {
>>       if (tv) {
>> -             struct timeval ktv;
>> -             do_gettimeofday(&ktv);
>> -             if (put_tv32(tv, &ktv))
>> +             struct timespec64 kts;
>> +
>> +             ktime_get_ts64(&kts);
> [...]
>
> But this syscall is supposed to use the realtime clock, no?  It seems
> like the correct substitute here is getnstimeofday64() (as the kernel-
> doc comment for do_gettimeofday() *almost* says).

It should be ktime_get_real_ts64(), thanks for noticing my mistake.

ktime_get_real_ts64() is the same as getnstimeofday64(), but I'm
trying to use the ktime_get* naming where possible.

        Arnd

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

end of thread, other threads:[~2017-11-08 14:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-07 14:09 [PATCH 1/2] alpha: osf_sys.c: fix put_tv32 regression Arnd Bergmann
2017-11-07 14:09 ` [PATCH 2/2] alpha: osf_sys.c: use timespec64 where appropriate Arnd Bergmann
2017-11-08  0:22   ` [Y2038] " Ben Hutchings
2017-11-08 14:54     ` Arnd Bergmann
2017-11-07 15:52 ` [PATCH 1/2] alpha: osf_sys.c: fix put_tv32 regression Al Viro
2017-11-07 16:03   ` Arnd Bergmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.