linux-kernel - Re: [PATCH] mm: fix possible cause of a page

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 for Android: free password hash cracker in your pocket

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

Date:	Wed, 4 May 2011 17:09:40 -0700
From:	Michel Lespinasse <walken@...gle.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Robert Święcki <robert@...ecki.net>,
	Hugh Dickins <hughd@...gle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Miklos Szeredi <miklos@...redi.hu>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Rik van Riel <riel@...hat.com>
Subject: Re: [PATCH] mm: fix possible cause of a page_mapped BUG

FYI, the attached code causes an infinite loop in kernels that have
the 95042f9eb7 commit:

#include <stdio.h>
#include <string.h>

#include <unistd.h>
#include <sys/syscall.h>
#include <linux/futex.h>

int *get_stack_guard(void)
{
  FILE *map;
  char buf[1000];

  map = fopen("/proc/self/maps", "r");
  if (!map)
    return NULL;
  while(fgets(buf, 1000, map)) {
    long a, b;
    char c[1000], d[1000], e[1000], f[1000], g[1000];
    if (sscanf(buf, "%lx-%lx %s %s %s %s %s", &a, &b, c, d, e, f, g) == 7 &&
        !strcmp(g, "[stack]")) {
      fclose(map);
      return (int *)(a - 4096);
    }
  }
  fclose(map);
  return NULL;
}

int main(void)
{
  int *uaddr = get_stack_guard();
  syscall(SYS_futex, uaddr, FUTEX_LOCK_PI_PRIVATE, 0, NULL, NULL, 0);
  return 0;
}

Linus, I am not sure as to what would be the preferred way to fix
this. One option could be to modify fault_in_user_writeable so that it
passes a non-NULL page pointer, and just does a put_page on it
afterwards. While this would work, this is kinda ugly and would slow
down futex operations somewhat. A more conservative alternative could
be to enable the guard page special case under an new GUP flag, but
this loses much of the elegance of your original proposal...

On Mon, Apr 18, 2011 at 2:15 PM, Michel Lespinasse <walken@...gle.com> wrote:
> This second patch looks more attractive than the first, but is also
> harder to prove correct. Hugh looked at all gup call sites and
> convinced himself that the change was safe, except for the
> fault_in_user_writeable() site in futex.c which he asked me to look
> at. I am worried that we would have an issue there, as places like
> futex_wake_op() or fixup_pi_state_owner() operate on user memory with
> page faults disabled, and expect fault_in_user_writeable() to set up
> the user page so that they can retry if the initial access failed.
> With this proposal, fault_in_user_writeable() would become inoperative
> when the  address is within the guard page; this could cause some
> malicious futex operation to create an infinite loop.

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
--
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/