Re: [PATCH] x86-64: Set siginfo and context on vsyscall emulation faults

[Linus Torvalds]

On Sat, Oct 22, 2011 at 12:01 AM, Andy Lutomirski <luto@xxxxxxxxxxxxxx> wrote:
>
> +static bool write_ok_or_segv(unsigned long ptr, size_t size)
> +{
> +       if (ptr == 0)
> +               return true;

Why is ptr==0 special? That makes no sense.

Also, this whole function makes the notion of setting the "sigsegv on
fault" flag much less interesting. It would be much better if
access_ok() (including the cases embedded in get_user/put_user/etc)
just did it right automatically for everything, rather than
special-casing it for just this.

I wonder if we could just make access_ok() use a trap instead of just
the regular compares (and then in the trap handler do the same logic
as in the page fault handler)? Sadly, the 'bounds' instruction doesn't
work for this (in 32-bit mode it does a *signed* compare, and in
64-bit mode it no longer exists), but something like that might.

That said, I think that your patch looks acceptable as a "let's fix
vsyscalls without doing the bigger change". But I really don't see why
ptr==0 would be special.

So I think your write_ok_or_segv() function should just be

   static bool write_ok_or_segv(unsigned long ptr, size_t size)
   {
      if (access_ok(ptr, size))
         return true;

       .. send signal ...

      return false;
   }

instead of that odd thing you have now.

           Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/