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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 22 Oct 2011 07:46:21 +0300
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Andy Lutomirski <luto@...capital.net>
Cc:	x86@...nel.org, Ingo Molnar <mingo@...e.hu>,
	richard -rw- weinberger <richard.weinberger@...il.com>,
	Adrian Bunk <bunk@...sta.de>,
	"H. Peter Anvin" <hpa@...ux.intel.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86-64: Set siginfo and context on vsyscall emulation faults

On Sat, Oct 22, 2011 at 12:01 AM, Andy Lutomirski <luto@...capital.net> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/