Re: passing NULL to clock_getres (VDSO): terminated by unexpected signal 11

[Thomas Gleixner <tglx@linutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 2052 bytes --]

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 <christophe.leroy@c-s.fr> wrote:
> > 
> > > Le 19/10/2019 à 21:18, Andreas Schwab a écrit :
> > > > On Okt 19 2019, Christophe Leroy <christophe.leroy@c-s.fr> 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