Re: passing NULL to clock_getres (VDSO): terminated by unexpected signal 11
Le 28/10/2019 à 16:46, Nathan Lynch a écrit :
Hi Christophe,
Christophe Leroy writes:
Hi Nathan,
While trying to switch powerpc VDSO to C version of gettimeofday(), I'm
getting the following kind of error with vdsotest:
passing NULL to clock_getres (VDSO): terminated by unexpected signal 11
Looking at commit a9446a906f52 ("lib/vdso/32: Remove inconsistent NULL
pointer checks"), it seems that signal 11 is expected when passing NULL
pointer.
Any plan to fix vdsotest ?
I'm afraid other work has kept me from following up on this promptly,
sorry. I've read the thread here:
https://lore.kernel.org/linuxppc-dev/87v9skcznp@igel.home/
And it looks like vdsotest does not need a fix (and in fact found a bug)
-- correct?
Yes that's correct, thanks.
Christophe
Re: passing NULL to clock_getres (VDSO): terminated by unexpected signal 11
Hi Christophe,
Christophe Leroy writes:
> Hi Nathan,
>
> While trying to switch powerpc VDSO to C version of gettimeofday(), I'm
> getting the following kind of error with vdsotest:
>
> passing NULL to clock_getres (VDSO): terminated by unexpected signal 11
>
> Looking at commit a9446a906f52 ("lib/vdso/32: Remove inconsistent NULL
> pointer checks"), it seems that signal 11 is expected when passing NULL
> pointer.
>
> Any plan to fix vdsotest ?
I'm afraid other work has kept me from following up on this promptly,
sorry. I've read the thread here:
https://lore.kernel.org/linuxppc-dev/87v9skcznp@igel.home/
And it looks like vdsotest does not need a fix (and in fact found a bug)
-- correct?
RE: passing NULL to clock_getres (VDSO): terminated by unexpected signal 11
From: Thomas Gleixner > Sent: 20 October 2019 20:53 > On Sun, 20 Oct 2019, Andreas Schwab wrote: > > On Okt 20 2019, Thomas Gleixner wrote: > > > > > POSIX does not mention anything about the validity of the pointer handed > > > to > > > clock_getres(). > > > > Sure it does: "If the argument res is not NULL, the resolution of the > > specified clock shall be stored in the location pointed to by res. If > > res is NULL, the clock resolution is not returned.". > > Sigh, that makes a lot of sense - NOT. > > But for the sake of making a non-sensical specification happy we can add a > NULL pointer check for this. The interesting question is what should be > returned in this case. The kernel returns EFAULT which is probably not > POSIX compliant either. The application won't see errno == EFAULT. EFAULT gets converted to SIGSEGV (probably) in the return-to-user code path. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: passing NULL to clock_getres (VDSO): terminated by unexpected signal 11
On Okt 20 2019, Thomas Gleixner wrote: > But for the sake of making a non-sensical specification happy we can add a > NULL pointer check for this. The interesting question is what should be > returned in this case. 0 if the clock id is valid, EINVAL otherwise. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different."
Re: passing NULL to clock_getres (VDSO): terminated by unexpected signal 11
On Sun, 20 Oct 2019, Andreas Schwab wrote: > On Okt 20 2019, Thomas Gleixner wrote: > > > POSIX does not mention anything about the validity of the pointer handed to > > clock_getres(). > > Sure it does: "If the argument res is not NULL, the resolution of the > specified clock shall be stored in the location pointed to by res. If > res is NULL, the clock resolution is not returned.". Sigh, that makes a lot of sense - NOT. But for the sake of making a non-sensical specification happy we can add a NULL pointer check for this. The interesting question is what should be returned in this case. The kernel returns EFAULT which is probably not POSIX compliant either. Patches are welcome. Thanks, tglx
Re: passing NULL to clock_getres (VDSO): terminated by unexpected signal 11
On Okt 20 2019, Thomas Gleixner wrote: > POSIX does not mention anything about the validity of the pointer handed to > clock_getres(). Sure it does: "If the argument res is not NULL, the resolution of the specified clock shall be stored in the location pointed to by res. If res is NULL, the clock resolution is not returned.". Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different."
Re: passing NULL to clock_getres (VDSO): terminated by unexpected signal 11
On Sun, 20 Oct 2019, Andreas Schwab wrote: > On Okt 20 2019, Thomas Gleixner wrote: > > > clock_getres(NULL) is hardly valid. > > Of course not, it lacks a parameter. You know exactly what I mean. > > So special casing > > > > clock_getres(clock, NULL); > > > > just to make a test case happy is a pointless exercise which does not make > > any sense at all. > > POSIX requires it to work. POSIX does not mention anything about the validity of the pointer handed to clock_getres(). https://pubs.opengroup.org/onlinepubs/9699919799/ "The clock_getres(), clock_gettime(), and clock_settime() functions shall fail if: [EINVAL] The clock_id argument does not specify a known clock." EINVAL is the only documented error code for clock_getres(). Again. The VDSO is not a syscall and therefore neither address space validation of the supplied pointer nor catching an invalid access works like it does in a syscall. The only invalid pointer it can catch is NULL, but that's ONE case out of a gazillion. So what's the value of catching NULL and only NULL? If you can come up with a solution to handle invalid pointers in the VDSO gracefully, then I'm surely all ears. If not then can you please explain why VDSO usage worked without pointer validation since the time VDSO was invented more than a decade ago and just now requires full invalid pointer handling? Thanks, tglx
Re: passing NULL to clock_getres (VDSO): terminated by unexpected signal 11
On Okt 20 2019, Thomas Gleixner wrote: > clock_getres(NULL) is hardly valid. Of course not, it lacks a parameter. > So special casing > > clock_getres(clock, NULL); > > just to make a test case happy is a pointless exercise which does not make > any sense at all. POSIX requires it to work. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different."
Re: passing NULL to clock_getres (VDSO): terminated by unexpected signal 11
On Sun, 20 Oct 2019, Christophe Leroy wrote:
> Adding Thomas to the discussion as the commit is from him.
>
> Le 20/10/2019 à 11:53, Andreas Schwab a écrit :
> > On Okt 20 2019, Christophe Leroy wrote:
> >
> > > Le 19/10/2019 à 21:18, Andreas Schwab a écrit :
> > > > On Okt 19 2019, Christophe Leroy wrote:
> > > >
> > > > > Hi Nathan,
> > > > >
> > > > > While trying to switch powerpc VDSO to C version of gettimeofday(),
> > > > > I'm
> > > > > getting the following kind of error with vdsotest:
> > > > >
> > > > > passing NULL to clock_getres (VDSO): terminated by unexpected signal
> > > > > 11
> > > > >
> > > > > Looking at commit a9446a906f52 ("lib/vdso/32: Remove inconsistent NULL
> > > > > pointer checks"), it seems that signal 11 is expected when passing
> > > > > NULL
> > > > > pointer.
> > > > >
> > > > > Any plan to fix vdsotest ?
> > > >
> > > > Passing NULL to clock_getres is valid, and required to return
> > > > successfully if the clock id is valid.
> > > >
> > >
> > > Do you mean the following commit is wrong ?
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/lib/vdso?id=a9446a906f52292c52ecbd5be78eaa4d8395756c
> >
> > If it causes a valid call to clock_getres to fail, then yes.
clock_getres(NULL) is hardly valid.
Fact is that 64bit did never check the pointer and unconditionally let the
NULL pointer dereference happen.
Also as documented in the change log, the vdso _cannot_ ever be fully
equivalent to the syscall.
The simple example:
struct timespec *ts = (struct timespec *) 0xFF;
clock_getres(clock, ts);
will fault in the VDSO, but not in the syscall.
VDSO can never ever guarantee that any of the clock_* functions will return
EFAULT if the provided pointer points outside of the accessible address
space, is mapped RO etc.
So special casing
clock_getres(clock, NULL);
just to make a test case happy is a pointless exercise which does not make
any sense at all.
Thanks,
tglx
Re: passing NULL to clock_getres (VDSO): terminated by unexpected signal 11
Adding Thomas to the discussion as the commit is from him.
Le 20/10/2019 à 11:53, Andreas Schwab a écrit :
On Okt 20 2019, Christophe Leroy wrote:
Le 19/10/2019 à 21:18, Andreas Schwab a écrit :
On Okt 19 2019, Christophe Leroy wrote:
Hi Nathan,
While trying to switch powerpc VDSO to C version of gettimeofday(), I'm
getting the following kind of error with vdsotest:
passing NULL to clock_getres (VDSO): terminated by unexpected signal 11
Looking at commit a9446a906f52 ("lib/vdso/32: Remove inconsistent NULL
pointer checks"), it seems that signal 11 is expected when passing NULL
pointer.
Any plan to fix vdsotest ?
Passing NULL to clock_getres is valid, and required to return
successfully if the clock id is valid.
Do you mean the following commit is wrong ?
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/lib/vdso?id=a9446a906f52292c52ecbd5be78eaa4d8395756c
If it causes a valid call to clock_getres to fail, then yes.
Andreas.
Re: passing NULL to clock_getres (VDSO): terminated by unexpected signal 11
On Okt 20 2019, Christophe Leroy wrote:
> Le 19/10/2019 à 21:18, Andreas Schwab a écrit :
>> On Okt 19 2019, Christophe Leroy wrote:
>>
>>> Hi Nathan,
>>>
>>> While trying to switch powerpc VDSO to C version of gettimeofday(), I'm
>>> getting the following kind of error with vdsotest:
>>>
>>> passing NULL to clock_getres (VDSO): terminated by unexpected signal 11
>>>
>>> Looking at commit a9446a906f52 ("lib/vdso/32: Remove inconsistent NULL
>>> pointer checks"), it seems that signal 11 is expected when passing NULL
>>> pointer.
>>>
>>> Any plan to fix vdsotest ?
>>
>> Passing NULL to clock_getres is valid, and required to return
>> successfully if the clock id is valid.
>>
>
> Do you mean the following commit is wrong ?
>
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/lib/vdso?id=a9446a906f52292c52ecbd5be78eaa4d8395756c
If it causes a valid call to clock_getres to fail, then yes.
Andreas.
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
Re: passing NULL to clock_getres (VDSO): terminated by unexpected signal 11
Le 19/10/2019 à 21:18, Andreas Schwab a écrit :
On Okt 19 2019, Christophe Leroy wrote:
Hi Nathan,
While trying to switch powerpc VDSO to C version of gettimeofday(), I'm
getting the following kind of error with vdsotest:
passing NULL to clock_getres (VDSO): terminated by unexpected signal 11
Looking at commit a9446a906f52 ("lib/vdso/32: Remove inconsistent NULL
pointer checks"), it seems that signal 11 is expected when passing NULL
pointer.
Any plan to fix vdsotest ?
Passing NULL to clock_getres is valid, and required to return
successfully if the clock id is valid.
Do you mean the following commit is wrong ?
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/lib/vdso?id=a9446a906f52292c52ecbd5be78eaa4d8395756c
Christophe
Re: passing NULL to clock_getres (VDSO): terminated by unexpected signal 11
On Okt 19 2019, Christophe Leroy wrote:
> Hi Nathan,
>
> While trying to switch powerpc VDSO to C version of gettimeofday(), I'm
> getting the following kind of error with vdsotest:
>
> passing NULL to clock_getres (VDSO): terminated by unexpected signal 11
>
> Looking at commit a9446a906f52 ("lib/vdso/32: Remove inconsistent NULL
> pointer checks"), it seems that signal 11 is expected when passing NULL
> pointer.
>
> Any plan to fix vdsotest ?
Passing NULL to clock_getres is valid, and required to return
successfully if the clock id is valid.
Andreas.
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
