Re: times(2) sys call bug?
On Fri, 2008-11-21 at 10:50 +0100, Gabriel Paubert wrote: > On Fri, Nov 21, 2008 at 08:03:06PM +1100, Paul Mackerras wrote: > > Gabriel Paubert writes: > > > > > On Fri, Nov 21, 2008 at 10:52:14AM +1100, Paul Mackerras wrote: > > > > Joakim Tjernlund writes: > > > > > > > > > This little hack changes the kernel sys call handling in an crude > > > > > way and then it works. Apperently the kernel thinks is an error if the > > > > > syscall returns a value between -_LAST_ERRNO and -1. > > > > > > > > Try this patch and let me if it fixes it. If it does I'll push it > > > > upstream. > > > > > > With your patch, you won't get EFAULT if you pass a bad > > > address, but a constant, time independent value, unless > > > I miss something. > > > > I think you are missing something, namely that I put the call to > > force_successful_syscall_return() AFTER the return -EFAULT. > > > > Indeed, it may be time to update the syscall documentation, saying > that you need to clear errno before the syscall and check errno > and not the return value since -1 is valid. And perhaps mention that times(NULL) never returns an error. And that times() is broken in 2.6.27 and earlier and needs the workaround posted earlier. Jocke ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: times(2) sys call bug?
On Fri, 2008-11-21 at 20:51 +1100, Paul Mackerras wrote: > Joakim Tjernlund writes: > > > > + force_successful_syscall_return(); > > > return (long) jiffies_64_to_clock_t(get_jiffies_64()); > > > > Why is 64 bits ops used here when you only use 32 bits? > > If HZ is 1000, jiffies_64_to_clock_t is going to divide jiffies by 10, > so we need to start with 64 bits in order to get the top few bits > of a 32-bit result correct. I see, thanks. > > > BTW, I think time(2) needs this: > > In principle you are correct, but it's only going to matter for a > little over an hour some time in the year 2106. :) I know, but I figured it should be fixed to serve as an template for other similar sys calls(not that I know of any offhand). Perhaps add it commented? Jocke ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: times(2) sys call bug?
Gabriel Paubert writes: > Who does this? I have spotted some errors in other places on > my man pages too, especially in the networking area (they were > correct once upon a time, but have not been updated). Michael Kerrisk <[EMAIL PROTECTED]>, I believe. Paul. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: times(2) sys call bug?
On Fri, 2008-11-21 at 10:31 +0100, Joakim Tjernlund wrote:
> On Fri, 2008-11-21 at 10:52 +1100, Paul Mackerras wrote:
> > Joakim Tjernlund writes:
> >
> > > This little hack changes the kernel sys call handling in an crude
> > > way and then it works. Apperently the kernel thinks is an error if the
> > > syscall returns a value between -_LAST_ERRNO and -1.
> >
> > Try this patch and let me if it fixes it. If it does I'll push it
> > upstream.
> >
> > Paul.
> [SNIP]
> > + force_successful_syscall_return();
> > return (long) jiffies_64_to_clock_t(get_jiffies_64());
>
> Why is 64 bits ops used here when you only use 32 bits?
>
> BTW, I think time(2) needs this:
>
> diff --git a/kernel/time.c b/kernel/time.c
[SNIP]
Oh, and compat needs fixing too.
For both my patches:
Signed-off-by: Joakim Tjernlund <[EMAIL PROTECTED]>
BTW, why is sys_time() impl. differently in compat:
do_gettimeofday(&tv);
vs.
get_seconds();
same for sys_times():
compat_jiffies_to_clock_t(jiffies);
vs.
jiffies_64_to_clock_t(get_jiffies_64());
diff --git a/kernel/compat.c b/kernel/compat.c
index 32c254a..c6346ec 100644
--- a/kernel/compat.c
+++ b/kernel/compat.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include
@@ -196,6 +197,7 @@ asmlinkage long compat_sys_times(struct compat_tms __user
*tbuf)
if (copy_to_user(tbuf, &tmp, sizeof(tmp)))
return -EFAULT;
}
+ force_successful_syscall_return();
return compat_jiffies_to_clock_t(jiffies);
}
@@ -850,8 +852,9 @@ asmlinkage long compat_sys_time(compat_time_t __user * tloc)
if (tloc) {
if (put_user(i,tloc))
- i = -EFAULT;
+ return -EFAULT;
}
+ force_successful_syscall_return();
return i;
}
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: times(2) sys call bug?
Joakim Tjernlund writes: > > + force_successful_syscall_return(); > > return (long) jiffies_64_to_clock_t(get_jiffies_64()); > > Why is 64 bits ops used here when you only use 32 bits? If HZ is 1000, jiffies_64_to_clock_t is going to divide jiffies by 10, so we need to start with 64 bits in order to get the top few bits of a 32-bit result correct. > BTW, I think time(2) needs this: In principle you are correct, but it's only going to matter for a little over an hour some time in the year 2106. :) Paul. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: times(2) sys call bug?
On Fri, Nov 21, 2008 at 08:03:06PM +1100, Paul Mackerras wrote: > Gabriel Paubert writes: > > > On Fri, Nov 21, 2008 at 10:52:14AM +1100, Paul Mackerras wrote: > > > Joakim Tjernlund writes: > > > > > > > This little hack changes the kernel sys call handling in an crude > > > > way and then it works. Apperently the kernel thinks is an error if the > > > > syscall returns a value between -_LAST_ERRNO and -1. > > > > > > Try this patch and let me if it fixes it. If it does I'll push it > > > upstream. > > > > With your patch, you won't get EFAULT if you pass a bad > > address, but a constant, time independent value, unless > > I miss something. > > I think you are missing something, namely that I put the call to > force_successful_syscall_return() AFTER the return -EFAULT. > Indeed, it may be time to update the syscall documentation, saying that you need to clear errno before the syscall and check errno and not the return value since -1 is valid. Who does this? I have spotted some errors in other places on my man pages too, especially in the networking area (they were correct once upon a time, but have not been updated). Regards, Gabriel ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: times(2) sys call bug?
On Fri, 2008-11-21 at 10:52 +1100, Paul Mackerras wrote:
> Joakim Tjernlund writes:
>
> > This little hack changes the kernel sys call handling in an crude
> > way and then it works. Apperently the kernel thinks is an error if the
> > syscall returns a value between -_LAST_ERRNO and -1.
>
> Try this patch and let me if it fixes it. If it does I'll push it
> upstream.
>
> Paul.
[SNIP]
> + force_successful_syscall_return();
> return (long) jiffies_64_to_clock_t(get_jiffies_64());
Why is 64 bits ops used here when you only use 32 bits?
BTW, I think time(2) needs this:
diff --git a/kernel/time.c b/kernel/time.c
index 6a08660..1627910 100644
--- a/kernel/time.c
+++ b/kernel/time.c
@@ -37,6 +37,7 @@
#include
#include
#include
+#include
#include
#include
@@ -65,8 +66,9 @@ asmlinkage long sys_time(time_t __user * tloc)
if (tloc) {
if (put_user(i,tloc))
- i = -EFAULT;
+ return -EFAULT;
}
+ force_successful_syscall_return();
return i;
}
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: times(2) sys call bug?
On Fri, 2008-11-21 at 20:03 +1100, Paul Mackerras wrote:
> Gabriel Paubert writes:
>
> > On Fri, Nov 21, 2008 at 10:52:14AM +1100, Paul Mackerras wrote:
> > > Joakim Tjernlund writes:
> > >
> > > > This little hack changes the kernel sys call handling in an crude
> > > > way and then it works. Apperently the kernel thinks is an error if the
> > > > syscall returns a value between -_LAST_ERRNO and -1.
> > >
> > > Try this patch and let me if it fixes it. If it does I'll push it
> > > upstream.
> >
> > With your patch, you won't get EFAULT if you pass a bad
> > address, but a constant, time independent value, unless
> > I miss something.
>
> I think you are missing something, namely that I put the call to
> force_successful_syscall_return() AFTER the return -EFAULT.
>
> You should get an EFAULT error if the address is bad, i.e. on return
> to userspace with cr0.SO = 1 and r3 = EFAULT (note, not -EFAULT). On
> a non-error return you should get cr0.SO = 0 and r3 containing the
> return value (even if it's -EFAULT). It's possible that glibc will
> stuff it up again after that but I hope not.
With your patch:
t1 = times((void*) 1);
if (t1 == -1) {
my_err = errno;
printf("Errno:%d, %s\n", my_err, strerror(my_err));
}
prints:
Errno:14, Bad address
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: times(2) sys call bug?
Gabriel Paubert writes: > On Fri, Nov 21, 2008 at 10:52:14AM +1100, Paul Mackerras wrote: > > Joakim Tjernlund writes: > > > > > This little hack changes the kernel sys call handling in an crude > > > way and then it works. Apperently the kernel thinks is an error if the > > > syscall returns a value between -_LAST_ERRNO and -1. > > > > Try this patch and let me if it fixes it. If it does I'll push it > > upstream. > > With your patch, you won't get EFAULT if you pass a bad > address, but a constant, time independent value, unless > I miss something. I think you are missing something, namely that I put the call to force_successful_syscall_return() AFTER the return -EFAULT. You should get an EFAULT error if the address is bad, i.e. on return to userspace with cr0.SO = 1 and r3 = EFAULT (note, not -EFAULT). On a non-error return you should get cr0.SO = 0 and r3 containing the return value (even if it's -EFAULT). It's possible that glibc will stuff it up again after that but I hope not. Paul. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: times(2) sys call bug?
On Fri, 2008-11-21 at 09:41 +0100, Gabriel Paubert wrote:
> On Fri, Nov 21, 2008 at 10:52:14AM +1100, Paul Mackerras wrote:
> > Joakim Tjernlund writes:
> >
> > > This little hack changes the kernel sys call handling in an crude
> > > way and then it works. Apperently the kernel thinks is an error if the
> > > syscall returns a value between -_LAST_ERRNO and -1.
> >
> > Try this patch and let me if it fixes it. If it does I'll push it
> > upstream.
>
> With your patch, you won't get EFAULT if you pass a bad
> address, but a constant, time independent value, unless
> I miss something.
Not so, look again:
asmlinkage long sys_times(struct tms __user * tbuf)
{
...
if (tbuf) {
...
if (copy_to_user(tbuf, &tmp, sizeof(struct tms)))
return -EFAULT;
}
force_successful_syscall_return();
return (long) jiffies_64_to_clock_t(get_jiffies_64());
}
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: times(2) sys call bug?
On Fri, Nov 21, 2008 at 10:52:14AM +1100, Paul Mackerras wrote: > Joakim Tjernlund writes: > > > This little hack changes the kernel sys call handling in an crude > > way and then it works. Apperently the kernel thinks is an error if the > > syscall returns a value between -_LAST_ERRNO and -1. > > Try this patch and let me if it fixes it. If it does I'll push it > upstream. With your patch, you won't get EFAULT if you pass a bad address, but a constant, time independent value, unless I miss something. Of course there are peoaple who claim that EFAULT is a bad idea to start with and that you should send a SIGSEGV instead, and I can see their point. But with the current implementation, it is a game that you can't win: any syscall that wants to return an arbitrary integer multiplexed with an error value is broken beyond repair, by design. Oh, well. Gabriel ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: times(2) sys call bug?
On Fri, 2008-11-21 at 10:52 +1100, Paul Mackerras wrote:
> Joakim Tjernlund writes:
>
> > This little hack changes the kernel sys call handling in an crude
> > way and then it works. Apperently the kernel thinks is an error if the
> > syscall returns a value between -_LAST_ERRNO and -1.
>
> Try this patch and let me if it fixes it. If it does I'll push it
> upstream.
>
It does fix the problem, thanks. You might want to do the same
to time(2)?
This workaround lets you get around the times(2) problem. Perhaps
you want to mention it in the commit msg:
static clock_t
our_times(void)/* Make times(2) behave rationally on Linux */
{
clock_t ret;
errno = 0;
ret = times(NULL);
if (errno != 0)
ret = (clock_t) (-errno);
return ret;
}
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: times(2) sys call bug?
Joakim Tjernlund writes: > This little hack changes the kernel sys call handling in an crude > way and then it works. Apperently the kernel thinks is an error if the > syscall returns a value between -_LAST_ERRNO and -1. Try this patch and let me if it fixes it. If it does I'll push it upstream. Paul. diff --git a/kernel/sys.c b/kernel/sys.c index 31deba8..1bf8c5c 100644 --- a/kernel/sys.c +++ b/kernel/sys.c @@ -33,6 +33,7 @@ #include #include #include +#include #include #include @@ -878,6 +879,7 @@ asmlinkage long sys_times(struct tms __user * tbuf) if (copy_to_user(tbuf, &tmp, sizeof(struct tms))) return -EFAULT; } + force_successful_syscall_return(); return (long) jiffies_64_to_clock_t(get_jiffies_64()); } ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: times(2) sys call bug?
On Thu, 2008-11-20 at 10:37 -0500, Josh Boyer wrote: > On Thu, 20 Nov 2008 16:09:16 +0100 > "Joakim Tjernlund" <[EMAIL PROTECTED]> wrote: > > > Why does the below program end up reporting -1 > > multiple seconds when times() wrap: > > http://sources.redhat.com/bugzilla/show_bug.cgi?id=5209 > > josh I see, but this is a new ppc kernel bug I think. This little hack changes the kernel sys call handling in an crude way and then it works. Apperently the kernel thinks is an error if the syscall returns a value between -_LAST_ERRNO and -1. Perhaps a known bug? diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S index 1cbbf70..72effde 100644 --- a/arch/powerpc/kernel/entry_32.S +++ b/arch/powerpc/kernel/entry_32.S @@ -278,7 +278,8 @@ ret_from_syscall: SYNC MTMSRD(r10) lwz r9,TI_FLAGS(r12) - li r8,-_LAST_ERRNO + //lir8,-_LAST_ERRNO + li r8,-2 andi. r0,r9,(_TIF_SYSCALL_T_OR_A|_TIF_SINGLESTEP|_TIF_USER_WORK_MASK|_TIF_PERSYSCALL_MASK) bne-syscall_exit_work cmplw 0,r3,r8 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: times(2) sys call bug?
On Thu, 20 Nov 2008 16:09:16 +0100 "Joakim Tjernlund" <[EMAIL PROTECTED]> wrote: > Why does the below program end up reporting -1 > multiple seconds when times() wrap: http://sources.redhat.com/bugzilla/show_bug.cgi?id=5209 josh ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
