> Running a setuid program that uses robust futexes would have had:
>   cred->euid != pcred->euid
>   cred->euid == pcred->uid
> so the old permissions check would allow it. I'm not aware of any setuid
> programs that use robust futexes, so this is just a preventative measure.

> (This patch is based on changes from grsecurity.)

> Signed-off-by: Kees Cook <keesc...@chromium.org>

> diff --git a/kernel/futex.c b/kernel/futex.c
> index 1614be2..439440d 100644
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -59,6 +59,7 @@
>  #include <linux/magic.h>
>  #include <linux/pid.h>
>  #include <linux/nsproxy.h>
> +#include <linux/ptrace.h>

>  #include <asm/futex.h>

> @@ -2443,40 +2444,29 @@ SYSCALL_DEFINE3(get_robust_list, int, pid,
>  {
>    struct robust_list_head __user *head;
>    unsigned long ret;
> -  const struct cred *cred = current_cred(), *pcred;
> +  struct task_struct *p;

>    if (!futex_cmpxchg_enabled)
>            return -ENOSYS;

> +  rcu_read_lock();
> +
> +  ret = -ESRCH;
>    if (!pid)
> -          head = current->robust_list;
> +          p = current;
>    else {
> -          struct task_struct *p;
> -
> -          ret = -ESRCH;
> -          rcu_read_lock();
>            p = find_task_by_vpid(pid);
>            if (!p)
>                    goto err_unlock;
> -          ret = -EPERM;
> -          pcred = __task_cred(p);
> -          /* If victim is in different user_ns, then uids are not
> -             comparable, so we must have CAP_SYS_PTRACE */
> -          if (cred->user->user_ns != pcred->user->user_ns) {
> -                  if (!ns_capable(pcred->user->user_ns, CAP_SYS_PTRACE))
> -                          goto err_unlock;
> -                  goto ok;
> -          }
> -          /* If victim is in same user_ns, then uids are comparable */
> -          if (cred->euid != pcred->euid &&
> -              cred->euid != pcred->uid &&
> -              !ns_capable(pcred->user->user_ns, CAP_SYS_PTRACE))
> -                  goto err_unlock;
> -ok:
> -          head = p->robust_list;
> -          rcu_read_unlock();
>    }

> +  ret = -EPERM;
> +  if (!ptrace_may_access(p, PTRACE_MODE_READ))
> +          goto err_unlock;
> +
> +  head = p->robust_list;
> +  rcu_read_unlock();
> +
>    if (put_user(sizeof(*head), len_ptr))
>            return -EFAULT;
>    return put_user(head, head_ptr);
> diff --git a/kernel/futex_compat.c b/kernel/futex_compat.c
> index 5f9e689..a9642d5 100644
> --- a/kernel/futex_compat.c
> +++ b/kernel/futex_compat.c
> @@ -10,6 +10,7 @@
>  #include <linux/compat.h>
>  #include <linux/nsproxy.h>
>  #include <linux/futex.h>
> +#include <linux/ptrace.h>

>  #include <asm/uaccess.h>

> @@ -136,40 +137,29 @@ compat_sys_get_robust_list(int pid, compat_uptr_t __user *head_ptr,
>  {
>    struct compat_robust_list_head __user *head;
>    unsigned long ret;
> -  const struct cred *cred = current_cred(), *pcred;
> +  struct task_struct *p;

>    if (!futex_cmpxchg_enabled)
>            return -ENOSYS;

> +  rcu_read_lock();
> +
> +  ret = -ESRCH;
>    if (!pid)
> -          head = current->compat_robust_list;
> +          p = current;
>    else {
> -          struct task_struct *p;
> -
> -          ret = -ESRCH;
> -          rcu_read_lock();
>            p = find_task_by_vpid(pid);
>            if (!p)
>                    goto err_unlock;
> -          ret = -EPERM;
> -          pcred = __task_cred(p);
> -          /* If victim is in different user_ns, then uids are not
> -             comparable, so we must have CAP_SYS_PTRACE */
> -          if (cred->user->user_ns != pcred->user->user_ns) {
> -                  if (!ns_capable(pcred->user->user_ns, CAP_SYS_PTRACE))
> -                          goto err_unlock;
> -                  goto ok;
> -          }
> -          /* If victim is in same user_ns, then uids are comparable */
> -          if (cred->euid != pcred->euid &&
> -              cred->euid != pcred->uid &&
> -              !ns_capable(pcred->user->user_ns, CAP_SYS_PTRACE))
> -                  goto err_unlock;
> -ok:
> -          head = p->compat_robust_list;
> -          rcu_read_unlock();
>    }

> +  ret = -EPERM;
> +  if (!ptrace_may_access(p, PTRACE_MODE_READ))
> +          goto err_unlock;
> +
> +  head = p->compat_robust_list;
> +  rcu_read_unlock();
> +
>    if (put_user(sizeof(*head), len_ptr))
>            return -EFAULT;
>    return put_user(ptr_to_compat(head), head_ptr);
> --
> 1.7.0.4

> > Running a setuid program that uses robust futexes would have had:
> >   cred->euid != pcred->euid
> >   cred->euid == pcred->uid
> > so the old permissions check would allow it. I'm not aware of any setuid
> > programs that use robust futexes, so this is just a preventative measure.

> > (This patch is based on changes from grsecurity.)

> > Signed-off-by: Kees Cook <keesc...@chromium.org>

> I like the change.  Much cleaner.  I'm not 100% sure though that
> there are no legitimate cases of robust futexes use which would now
> be forbidden.  (Explicitly cc:ing Ingo)

>> Quoting Kees Cook (keesc...@chromium.org):
>> > It was possible to extract the robust list head address from a setuid
>> > process if it had used set_robust_list(), allowing an ASLR info leak. This
>> > changes the permission checks to be the same as those used for similar
>> > info that comes out of /proc.

>> > Running a setuid program that uses robust futexes would have had:
>> >   cred->euid != pcred->euid
>> >   cred->euid == pcred->uid
>> > so the old permissions check would allow it. I'm not aware of any setuid
>> > programs that use robust futexes, so this is just a preventative measure.

>> > (This patch is based on changes from grsecurity.)

>> > Signed-off-by: Kees Cook <keesc...@chromium.org>

>> I like the change.  Much cleaner.  I'm not 100% sure though that
>> there are no legitimate cases of robust futexes use which would now
>> be forbidden.  (Explicitly cc:ing Ingo)

> get_robust_list is not necessary for robust futexes. There is no
> reference to get_robust_list in glibc.

> I really wonder why we have this syscall at all.

> >> Quoting Kees Cook (keesc...@chromium.org):
> >> > It was possible to extract the robust list head address from a setuid
> >> > process if it had used set_robust_list(), allowing an ASLR info leak. This
> >> > changes the permission checks to be the same as those used for similar
> >> > info that comes out of /proc.

> >> > Running a setuid program that uses robust futexes would have had:
> >> >   cred->euid != pcred->euid
> >> >   cred->euid == pcred->uid
> >> > so the old permissions check would allow it. I'm not aware of any setuid
> >> > programs that use robust futexes, so this is just a preventative measure.

> >> > (This patch is based on changes from grsecurity.)

> >> > Signed-off-by: Kees Cook <keesc...@chromium.org>

> >> I like the change.  Much cleaner.  I'm not 100% sure though that
> >> there are no legitimate cases of robust futexes use which would now
> >> be forbidden.  (Explicitly cc:ing Ingo)

> > get_robust_list is not necessary for robust futexes. There is no
> > reference to get_robust_list in glibc.

> > I really wonder why we have this syscall at all.

> The documentation I found yesterday while looking at this was:
> http://linux.die.net/man/2/get_robust_list

> Which says "The system call is only available for debugging
> purposes and is not needed for normal operations. Both system
> calls are not available to application programs as functions;
> they can be called using the syscall(3) function."

> Dropping the syscall entirely would certainly make it secure.
> ;)

> > The documentation I found yesterday while looking at this was:
> > http://linux.die.net/man/2/get_robust_list

> > Which says "The system call is only available for debugging
> > purposes and is not needed for normal operations. Both system
> > calls are not available to application programs as functions;
> > they can be called using the syscall(3) function."

> > Dropping the syscall entirely would certainly make it secure.
> > ;)

> The thinking was API completeness. In general it's possible for
> a sufficiently privileged task to figure out all the state of a
> task. We can query timers, fds - the robust list is such a
> resource as well. The information leakage was obviously not
> intended.

> diff --git a/kernel/futex.c b/kernel/futex.c
> index 439440d..545d7a3 100644
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -2449,6 +2449,8 @@ SYSCALL_DEFINE3(get_robust_list, int, pid,
>    if (!futex_cmpxchg_enabled)
>            return -ENOSYS;

> +  WARN_ONCE(1, "deprecated: get_robust_list will be deleted in 2013.\n");
> +
>    rcu_read_lock();

>    ret = -ESRCH;
> diff --git a/kernel/futex_compat.c b/kernel/futex_compat.c
> index a9642d5..83e368b 100644
> --- a/kernel/futex_compat.c
> +++ b/kernel/futex_compat.c
> @@ -142,6 +142,8 @@ compat_sys_get_robust_list(int pid, compat_uptr_t __user *head_ptr,
>    if (!futex_cmpxchg_enabled)
>            return -ENOSYS;

> +  WARN_ONCE(1, "deprecated: get_robust_list will be deleted in 2013.\n");
> +
>    rcu_read_lock();

>    ret = -ESRCH;
> --
> 1.7.0.4

> --
> Kees Cook
> ChromeOS Security

>> Notify get_robust_list users that the syscall is going away.

> That want's an entry in Documentation/feature-removal-schedule.txt as
> well.

> diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
> index 4bfd982..e3bf119 100644
> --- a/Documentation/feature-removal-schedule.txt
> +++ b/Documentation/feature-removal-schedule.txt
> @@ -543,3 +543,13 @@ When: 3.5
>  Why:      The old kmap_atomic() with two arguments is deprecated, we only
>    keep it for backward compatibility for few cycles and then drop it.
>  Who:      Cong Wang <amw...@redhat.com>
> +
> +----------------------------
> +
> +What:     get_robust_list syscall
> +When:     2013
> +Why:      There appear to be no production users of the get_robust_list syscall,
> +  and it runs the risk of leaking address locations, allowing the bypass
> +  of ASLR. It was only ever intended for debugging, so it should be
> +  removed.
> +Who:      Kees Cook <keesc...@chromium.org>
> diff --git a/kernel/futex.c b/kernel/futex.c
> index d701be5..e2b0fb9 100644
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -2449,6 +2449,8 @@ SYSCALL_DEFINE3(get_robust_list, int, pid,
>    if (!futex_cmpxchg_enabled)
>            return -ENOSYS;

> +  WARN_ONCE(1, "deprecated: get_robust_list will be deleted in 2013.\n");
> +
>    rcu_read_lock();

>    ret = -ESRCH;
> diff --git a/kernel/futex_compat.c b/kernel/futex_compat.c
> index a9642d5..83e368b 100644
> --- a/kernel/futex_compat.c
> +++ b/kernel/futex_compat.c
> @@ -142,6 +142,8 @@ compat_sys_get_robust_list(int pid, compat_uptr_t __user *head_ptr,
>    if (!futex_cmpxchg_enabled)
>            return -ENOSYS;

> +  WARN_ONCE(1, "deprecated: get_robust_list will be deleted in 2013.\n");
> +
>    rcu_read_lock();

>    ret = -ESRCH;

>> Notify get_robust_list users that the syscall is going away.

> Has anyone asked the question if the folks working on checkpoint/restart
> are going to need this.

> This seems like important information to know if you want to checkpoint
> a process.

> Suggested-by: Thomas Gleixner <t...@linutronix.de>
> Signed-off-by: Kees Cook <keesc...@chromium.org>
> ---
> v2:
>  - add note to feature-removal-schedule.txt.
> ---
>  Documentation/feature-removal-schedule.txt |   10 ++++++++++
>  kernel/futex.c                             |    2 ++
>  kernel/futex_compat.c                      |    2 ++
>  3 files changed, 14 insertions(+), 0 deletions(-)

> diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
> index 4bfd982..e3bf119 100644
> --- a/Documentation/feature-removal-schedule.txt
> +++ b/Documentation/feature-removal-schedule.txt
> @@ -543,3 +543,13 @@ When:      3.5
>  Why:   The old kmap_atomic() with two arguments is deprecated, we only
>        keep it for backward compatibility for few cycles and then drop it.
>  Who:   Cong Wang <amw...@redhat.com>
> +
> +----------------------------
> +
> +What:  get_robust_list syscall
> +When:  2013
> +Why:   There appear to be no production users of the get_robust_list syscall,
> +       and it runs the risk of leaking address locations, allowing the bypass
> +       of ASLR. It was only ever intended for debugging, so it should be
> +       removed.
> +Who:   Kees Cook <keesc...@chromium.org>
> diff --git a/kernel/futex.c b/kernel/futex.c
> index d701be5..e2b0fb9 100644
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -2449,6 +2449,8 @@ SYSCALL_DEFINE3(get_robust_list, int, pid,
>        if (!futex_cmpxchg_enabled)
>                return -ENOSYS;

> +       WARN_ONCE(1, "deprecated: get_robust_list will be deleted in 2013.\n");
> +

> Do you really need WARN_ONCE?  It's going to spew a backtrace if this
> is called, and that is going to cause various auto-bug reporters to file
> bugs as well.  

>> > The documentation I found yesterday while looking at this was:
>> > http://linux.die.net/man/2/get_robust_list

>> > Which says "The system call is only available for debugging
>> > purposes and is not needed for normal operations. Both system
>> > calls are not available to application programs as functions;
>> > they can be called using the syscall(3) function."

>> > Dropping the syscall entirely would certainly make it secure.
>> > ;)

>> The thinking was API completeness. In general it's possible for
>> a sufficiently privileged task to figure out all the state of a
>> task. We can query timers, fds - the robust list is such a
>> resource as well. The information leakage was obviously not
>> intended.

> So I think it's safe to take Kees' patch as is. On top of that we
> should add a WARN_ONCE when the syscall is invoked and schedule the
> sucker for removal.

> >> > The documentation I found yesterday while looking at this was:
> >> > http://linux.die.net/man/2/get_robust_list

> >> > Which says "The system call is only available for debugging
> >> > purposes and is not needed for normal operations. Both system
> >> > calls are not available to application programs as functions;
> >> > they can be called using the syscall(3) function."

> >> > Dropping the syscall entirely would certainly make it secure.
> >> > ;)

> >> The thinking was API completeness. In general it's possible for
> >> a sufficiently privileged task to figure out all the state of a
> >> task. We can query timers, fds - the robust list is such a
> >> resource as well. The information leakage was obviously not
> >> intended.

> > So I think it's safe to take Kees' patch as is. On top of that we
> > should add a WARN_ONCE when the syscall is invoked and schedule the
> > sucker for removal.

> Can someone claim the first patch? It looks like not everyone agrees
> about removal, but I'd like to see at least the first one get in. :)

> > Notify get_robust_list users that the syscall is going away.

> Has anyone asked the question if the folks working on checkpoint/restart
> are going to need this.

> This seems like important information to know if you want to checkpoint
> a process.

> Eric

> > Suggested-by: Thomas Gleixner <t...@linutronix.de>
> > Signed-off-by: Kees Cook <keesc...@chromium.org>
> > ---
> > v2:
> >  - add note to feature-removal-schedule.txt.
> > ---
> >  Documentation/feature-removal-schedule.txt |   10 ++++++++++
> >  kernel/futex.c                             |    2 ++
> >  kernel/futex_compat.c                      |    2 ++
> >  3 files changed, 14 insertions(+), 0 deletions(-)

> > diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
> > index 4bfd982..e3bf119 100644
> > --- a/Documentation/feature-removal-schedule.txt
> > +++ b/Documentation/feature-removal-schedule.txt
> > @@ -543,3 +543,13 @@ When:    3.5
> >  Why: The old kmap_atomic() with two arguments is deprecated, we only
> >       keep it for backward compatibility for few cycles and then drop it.
> >  Who: Cong Wang <amw...@redhat.com>
> > +
> > +----------------------------
> > +
> > +What:        get_robust_list syscall
> > +When:        2013
> > +Why: There appear to be no production users of the get_robust_list syscall,
> > +     and it runs the risk of leaking address locations, allowing the bypass
> > +     of ASLR. It was only ever intended for debugging, so it should be
> > +     removed.

>>> Notify get_robust_list users that the syscall is going away.

>> Has anyone asked the question if the folks working on checkpoint/restart
>> are going to need this.

>> This seems like important information to know if you want to checkpoint
>> a process.

> I have no idea if the CRIU and DMTCP folks care about this. I've added
> some folks related to those projects to the Cc list.

>> Eric

>>> Suggested-by: Thomas Gleixner <t...@linutronix.de>
>>> Signed-off-by: Kees Cook <keesc...@chromium.org>
>>> ---
>>> v2:
>>>  - add note to feature-removal-schedule.txt.
>>> ---
>>>  Documentation/feature-removal-schedule.txt |   10 ++++++++++
>>>  kernel/futex.c                             |    2 ++
>>>  kernel/futex_compat.c                      |    2 ++
>>>  3 files changed, 14 insertions(+), 0 deletions(-)

>>> diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
>>> index 4bfd982..e3bf119 100644
>>> --- a/Documentation/feature-removal-schedule.txt
>>> +++ b/Documentation/feature-removal-schedule.txt
>>> @@ -543,3 +543,13 @@ When: 3.5
>>>  Why:      The old kmap_atomic() with two arguments is deprecated, we only
>>>    keep it for backward compatibility for few cycles and then drop it.
>>>  Who:      Cong Wang <amw...@redhat.com>
>>> +
>>> +----------------------------
>>> +
>>> +What:     get_robust_list syscall
>>> +When:     2013
>>> +Why:      There appear to be no production users of the get_robust_list syscall,
>>> +  and it runs the risk of leaking address locations, allowing the bypass
>>> +  of ASLR. It was only ever intended for debugging, so it should be
>>> +  removed.

> So I've looked in glibc, gdb, and DMTCP. The description of the intended
> use of get_robust_list() is accurate. However the benefit of ASLR is
> less clear when it comes to the robust list. In glibc the robust list is
> only used from NPTL. The robust list head is in struct pthread which can be
> obtained from pthread_self() anyway. Thus I think ASLR doesn't really help
> obfuscate the robust futex list unless the program is using robust futexes
> without the aid of glibc.

> Cheers,
>    -Matt Helsley

> .

> > > Notify get_robust_list users that the syscall is going away.

> > Has anyone asked the question if the folks working on checkpoint/restart
> > are going to need this.

> > This seems like important information to know if you want to checkpoint
> > a process.

> I have no idea if the CRIU and DMTCP folks care about this. I've added
> some folks related to those projects to the Cc list.

> > Eric

> > > Suggested-by: Thomas Gleixner <t...@linutronix.de>
> > > Signed-off-by: Kees Cook <keesc...@chromium.org>
> > > ---
> > > v2:
> > >  - add note to feature-removal-schedule.txt.
> > > ---
> > >  Documentation/feature-removal-schedule.txt |   10 ++++++++++
> > >  kernel/futex.c                             |    2 ++
> > >  kernel/futex_compat.c                      |    2 ++
> > >  3 files changed, 14 insertions(+), 0 deletions(-)

> > > diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
> > > index 4bfd982..e3bf119 100644
> > > --- a/Documentation/feature-removal-schedule.txt
> > > +++ b/Documentation/feature-removal-schedule.txt
> > > @@ -543,3 +543,13 @@ When:       3.5
> > >  Why:    The old kmap_atomic() with two arguments is deprecated, we only
> > >          keep it for backward compatibility for few cycles and then drop it.
> > >  Who:    Cong Wang <amw...@redhat.com>
> > > +
> > > +----------------------------
> > > +
> > > +What:   get_robust_list syscall
> > > +When:   2013
> > > +Why:    There appear to be no production users of the get_robust_list syscall,
> > > +        and it runs the risk of leaking address locations, allowing the bypass
> > > +        of ASLR. It was only ever intended for debugging, so it should be
> > > +        removed.

> So I've looked in glibc, gdb, and DMTCP. The description of the intended
> use of get_robust_list() is accurate. However the benefit of ASLR is
> less clear when it comes to the robust list. In glibc the robust list is
> only used from NPTL. The robust list head is in struct pthread which can be
> obtained from pthread_self() anyway. Thus I think ASLR doesn't really help
> obfuscate the robust futex list unless the program is using robust futexes
> without the aid of glibc.

> Cheers,
>    -Matt Helsley

> futex: Do not leak robust list to unprivileged process

> It was possible to extract the robust list head address from a setuid
> process if it had used set_robust_list(), allowing an ASLR info leak. This
> changes the permission checks to be the same as those used for similar
> info that comes out of /proc.

> Running a setuid program that uses robust futexes would have had:
>   cred->euid != pcred->euid
>   cred->euid == pcred->uid
> so the old permissions check would allow it. I'm not aware of any setuid
> programs that use robust futexes, so this is just a preventative measure.

> Signed-off-by: Kees Cook <keesc...@chromium.org>
> Cc: Darren Hart <dvh...@linux.intel.com>
> Cc: Peter Zijlstra <a.p.zijls...@chello.nl>
> Cc: Jiri Kosina <jkos...@suse.cz>
> Cc: Eric W. Biederman <ebied...@xmission.com>
> Cc: David Howells <dhowe...@redhat.com>
> Cc: Serge E. Hallyn <serge.hal...@canonical.com>
> Cc: kernel-harden...@lists.openwall.com
> Cc: spen...@grsecurity.net
> Link: http://lkml.kernel.org/r/20120319231253.GA20...@www.outflux.net
> Signed-off-by: Thomas Gleixner <t...@linutronix.de>
> ---
>  kernel/futex.c        |   36 +++++++++++++-----------------------
>  kernel/futex_compat.c |   36 +++++++++++++-----------------------
>  2 files changed, 26 insertions(+), 46 deletions(-)

> diff --git a/kernel/futex.c b/kernel/futex.c
> index 72efa1e..d701be5 100644
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -59,6 +59,7 @@
>  #include <linux/magic.h>
>  #include <linux/pid.h>
>  #include <linux/nsproxy.h>
> +#include <linux/ptrace.h>

>  #include <asm/futex.h>

> @@ -2443,40 +2444,29 @@ SYSCALL_DEFINE3(get_robust_list, int, pid,
>  {
>    struct robust_list_head __user *head;
>    unsigned long ret;
> -  const struct cred *cred = current_cred(), *pcred;
> +  struct task_struct *p;

>    if (!futex_cmpxchg_enabled)
>            return -ENOSYS;

> +  rcu_read_lock();
> +
> +  ret = -ESRCH;
>    if (!pid)
> -          head = current->robust_list;
> +          p = current;
>    else {
> -          struct task_struct *p;
> -
> -          ret = -ESRCH;
> -          rcu_read_lock();
>            p = find_task_by_vpid(pid);
>            if (!p)
>                    goto err_unlock;
> -          ret = -EPERM;
> -          pcred = __task_cred(p);
> -          /* If victim is in different user_ns, then uids are not
> -             comparable, so we must have CAP_SYS_PTRACE */
> -          if (cred->user->user_ns != pcred->user->user_ns) {
> -                  if (!ns_capable(pcred->user->user_ns, CAP_SYS_PTRACE))
> -                          goto err_unlock;
> -                  goto ok;
> -          }
> -          /* If victim is in same user_ns, then uids are comparable */
> -          if (cred->euid != pcred->euid &&
> -              cred->euid != pcred->uid &&
> -              !ns_capable(pcred->user->user_ns, CAP_SYS_PTRACE))
> -                  goto err_unlock;
> -ok:
> -          head = p->robust_list;
> -          rcu_read_unlock();
>    }

> +  ret = -EPERM;
> +  if (!ptrace_may_access(p, PTRACE_MODE_READ))
> +          goto err_unlock;
> +
> +  head = p->robust_list;
> +  rcu_read_unlock();
> +
>    if (put_user(sizeof(*head), len_ptr))
>            return -EFAULT;
>    return put_user(head, head_ptr);
> diff --git a/kernel/futex_compat.c b/kernel/futex_compat.c
> index 5f9e689..a9642d5 100644
> --- a/kernel/futex_compat.c
> +++ b/kernel/futex_compat.c
> @@ -10,6 +10,7 @@
>  #include <linux/compat.h>
>  #include <linux/nsproxy.h>
>  #include <linux/futex.h>
> +#include <linux/ptrace.h>

>  #include <asm/uaccess.h>

> @@ -136,40 +137,29 @@ compat_sys_get_robust_list(int pid, compat_uptr_t __user *head_ptr,
>  {
>    struct compat_robust_list_head __user *head;
>    unsigned long ret;
> -  const struct cred *cred = current_cred(), *pcred;
> +  struct task_struct *p;

>    if (!futex_cmpxchg_enabled)
>            return -ENOSYS;

> +  rcu_read_lock();
> +
> +  ret = -ESRCH;
>    if (!pid)
> -          head = current->compat_robust_list;
> +          p = current;
>    else {
> -          struct task_struct *p;
> -
> -          ret = -ESRCH;
> -          rcu_read_lock();
>            p = find_task_by_vpid(pid);
>            if (!p)
>                    goto err_unlock;
> -          ret = -EPERM;
> -          pcred = __task_cred(p);
> -          /* If victim is in different user_ns, then uids are not
> -             comparable, so we must have CAP_SYS_PTRACE */
> -          if (cred->user->user_ns != pcred->user->user_ns) {
> -                  if (!ns_capable(pcred->user->user_ns, CAP_SYS_PTRACE))
> -                          goto err_unlock;
> -                  goto ok;
> -          }
> -          /* If victim is in same user_ns, then uids are comparable */
> -          if (cred->euid != pcred->euid &&
> -              cred->euid != pcred->uid &&
> -              !ns_capable(pcred->user->user_ns, CAP_SYS_PTRACE))
> -                  goto err_unlock;
> -ok:
> -          head = p->compat_robust_list;
> -          rcu_read_unlock();
>    }

> +  ret = -EPERM;
> +  if (!ptrace_may_access(p, PTRACE_MODE_READ))
> +          goto err_unlock;
> +
> +  head = p->compat_robust_list;
> +  rcu_read_unlock();
> +
>    if (put_user(sizeof(*head), len_ptr))
>            return -EFAULT;
>    return put_user(ptr_to_compat(head), head_ptr);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

> > futex: Do not leak robust list to unprivileged process

> > It was possible to extract the robust list head address from a setuid
> > process if it had used set_robust_list(), allowing an ASLR info leak. This
> > changes the permission checks to be the same as those used for similar
> > info that comes out of /proc.

> > Running a setuid program that uses robust futexes would have had:
> >   cred->euid != pcred->euid
> >   cred->euid == pcred->uid
> > so the old permissions check would allow it. I'm not aware of any setuid
> > programs that use robust futexes, so this is just a preventative measure.

> I'm not sure this change prevents the unprivileged process.
> Please refer to LTP test, recently I saw that this change broke
> the following test.

> https://github.com/linux-test-project/ltp/blob/master/testcases/kerne...
>            if (seteuid(1) == -1)
>                    tst_brkm(TBROK|TERRNO, cleanup, "seteuid(1) failed");

>            TEST(retval = syscall(__NR_get_robust_list, 1,
>                                  (struct robust_list_head *)&head,
>                                  &len_ptr));

> We set the euid to an unprivileged user, and expect to FAIL with EPERM,
> without this patch, it FAIL as we expected, but with it, this call succeed.

> Thanks in advance,
> Wanlong Gao

> > (This patch is based on changes from grsecurity.)

> > Signed-off-by: Kees Cook <keesc...@chromium.org>
> > Cc: Darren Hart <dvh...@linux.intel.com>
> > Cc: Peter Zijlstra <a.p.zijls...@chello.nl>
> > Cc: Jiri Kosina <jkos...@suse.cz>
> > Cc: Eric W. Biederman <ebied...@xmission.com>
> > Cc: David Howells <dhowe...@redhat.com>
> > Cc: Serge E. Hallyn <serge.hal...@canonical.com>
> > Cc: kernel-harden...@lists.openwall.com
> > Cc: spen...@grsecurity.net
> > Link: http://lkml.kernel.org/r/20120319231253.GA20...@www.outflux.net
> > Signed-off-by: Thomas Gleixner <t...@linutronix.de>
> > ---
> >  kernel/futex.c        |   36 +++++++++++++-----------------------
> >  kernel/futex_compat.c |   36 +++++++++++++-----------------------
> >  2 files changed, 26 insertions(+), 46 deletions(-)

> > diff --git a/kernel/futex.c b/kernel/futex.c
> > index 72efa1e..d701be5 100644
> > --- a/kernel/futex.c
> > +++ b/kernel/futex.c
> > @@ -59,6 +59,7 @@
> >  #include <linux/magic.h>
> >  #include <linux/pid.h>
> >  #include <linux/nsproxy.h>
> > +#include <linux/ptrace.h>

> >  #include <asm/futex.h>

> > @@ -2443,40 +2444,29 @@ SYSCALL_DEFINE3(get_robust_list, int, pid,
> >  {
> >       struct robust_list_head __user *head;
> >       unsigned long ret;
> > -     const struct cred *cred = current_cred(), *pcred;
> > +     struct task_struct *p;

> >       if (!futex_cmpxchg_enabled)
> >               return -ENOSYS;

> > +     rcu_read_lock();
> > +
> > +     ret = -ESRCH;
> >       if (!pid)
> > -             head = current->robust_list;
> > +             p = current;
> >       else {
> > -             struct task_struct *p;
> > -
> > -             ret = -ESRCH;
> > -             rcu_read_lock();
> >               p = find_task_by_vpid(pid);
> >               if (!p)
> >                       goto err_unlock;
> > -             ret = -EPERM;
> > -             pcred = __task_cred(p);
> > -             /* If victim is in different user_ns, then uids are not
> > -                comparable, so we must have CAP_SYS_PTRACE */
> > -             if (cred->user->user_ns != pcred->user->user_ns) {
> > -                     if (!ns_capable(pcred->user->user_ns, CAP_SYS_PTRACE))
> > -                             goto err_unlock;
> > -                     goto ok;
> > -             }
> > -             /* If victim is in same user_ns, then uids are comparable */
> > -             if (cred->euid != pcred->euid &&
> > -                 cred->euid != pcred->uid &&
> > -                 !ns_capable(pcred->user->user_ns, CAP_SYS_PTRACE))
> > -                     goto err_unlock;
> > -ok:
> > -             head = p->robust_list;
> > -             rcu_read_unlock();
> >       }

> > +     ret = -EPERM;
> > +     if (!ptrace_may_access(p, PTRACE_MODE_READ))
> > +             goto err_unlock;
> > +
> > +     head = p->robust_list;
> > +     rcu_read_unlock();
> > +
> >       if (put_user(sizeof(*head), len_ptr))
> >               return -EFAULT;
> >       return put_user(head, head_ptr);
> > diff --git a/kernel/futex_compat.c b/kernel/futex_compat.c
> > index 5f9e689..a9642d5 100644
> > --- a/kernel/futex_compat.c
> > +++ b/kernel/futex_compat.c
> > @@ -10,6 +10,7 @@
> >  #include <linux/compat.h>
> >  #include <linux/nsproxy.h>
> >  #include <linux/futex.h>
> > +#include <linux/ptrace.h>

> >  #include <asm/uaccess.h>

> > @@ -136,40 +137,29 @@ compat_sys_get_robust_list(int pid, compat_uptr_t __user *head_ptr,
> >  {
> >       struct compat_robust_list_head __user *head;
> >       unsigned long ret;
> > -     const struct cred *cred = current_cred(), *pcred;
> > +     struct task_struct *p;

> >       if (!futex_cmpxchg_enabled)
> >               return -ENOSYS;

> > +     rcu_read_lock();
> > +
> > +     ret = -ESRCH;
> >       if (!pid)
> > -             head = current->compat_robust_list;
> > +             p = current;
> >       else {
> > -             struct task_struct *p;
> > -
> > -             ret = -ESRCH;
> > -             rcu_read_lock();
> >               p = find_task_by_vpid(pid);
> >               if (!p)
> >                       goto err_unlock;
> > -             ret = -EPERM;
> > -             pcred = __task_cred(p);
> > -             /* If victim is in different user_ns, then uids are not
> > -                comparable, so we must have CAP_SYS_PTRACE */
> > -             if (cred->user->user_ns != pcred->user->user_ns) {
> > -                     if (!ns_capable(pcred->user->user_ns, CAP_SYS_PTRACE))
> > -                             goto err_unlock;
> > -                     goto ok;
> > -             }
> > -             /* If victim is in same user_ns, then uids are comparable */
> > -             if (cred->euid != pcred->euid &&
> > -                 cred->euid != pcred->uid &&
> > -                 !ns_capable(pcred->user->user_ns, CAP_SYS_PTRACE))
> > -                     goto err_unlock;
> > -ok:
> > -             head = p->compat_robust_list;
> > -             rcu_read_unlock();
> >       }

> > +     ret = -EPERM;
> > +     if (!ptrace_may_access(p, PTRACE_MODE_READ))
> > +             goto err_unlock;
> > +
> > +     head = p->compat_robust_list;
> > +     rcu_read_unlock();
> > +
> >       if (put_user(sizeof(*head), len_ptr))
> >               return -EFAULT;
> >       return put_user(ptr_to_compat(head), head_ptr);
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/

>>> futex: Do not leak robust list to unprivileged process

>>> It was possible to extract the robust list head address from a setuid
>>> process if it had used set_robust_list(), allowing an ASLR info leak. This
>>> changes the permission checks to be the same as those used for similar
>>> info that comes out of /proc.

>>> Running a setuid program that uses robust futexes would have had:
>>>   cred->euid != pcred->euid
>>>   cred->euid == pcred->uid
>>> so the old permissions check would allow it. I'm not aware of any setuid
>>> programs that use robust futexes, so this is just a preventative measure.

>> I'm not sure this change prevents the unprivileged process.
>> Please refer to LTP test, recently I saw that this change broke
>> the following test.

>> https://github.com/linux-test-project/ltp/blob/master/testcases/kerne...
>>                if (seteuid(1) == -1)
>>                        tst_brkm(TBROK|TERRNO, cleanup, "seteuid(1) failed");

>>                TEST(retval = syscall(__NR_get_robust_list, 1,
>>                                      (struct robust_list_head *)&head,
>>                                      &len_ptr));

>> We set the euid to an unprivileged user, and expect to FAIL with EPERM,
>> without this patch, it FAIL as we expected, but with it, this call succeed.

> This relates to a question I asked - I believe in this thread, maybe in
> another thread - about ptrace_may_access.  That code goes back further than
> our git history, and for so long has used current->uid and ->gid, not
> euid and gid, for permission checks.  I asked if that's what we really
> want, but at the same am not sure we want to change something that's
> been like that for so long.

> But that's why it succeeded - you changed your euid, not your uid.

>> Seems that we leaked the check of (cred->euid == pcred->euid && cred->euid == pcred->uid),
>> I'm not sure which one is right, can you please give an explanation?

>> Thanks in advance,
>> Wanlong Gao

>>> (This patch is based on changes from grsecurity.)

>>> Signed-off-by: Kees Cook <keesc...@chromium.org>
>>> Cc: Darren Hart <dvh...@linux.intel.com>
>>> Cc: Peter Zijlstra <a.p.zijls...@chello.nl>
>>> Cc: Jiri Kosina <jkos...@suse.cz>
>>> Cc: Eric W. Biederman <ebied...@xmission.com>
>>> Cc: David Howells <dhowe...@redhat.com>
>>> Cc: Serge E. Hallyn <serge.hal...@canonical.com>
>>> Cc: kernel-harden...@lists.openwall.com
>>> Cc: spen...@grsecurity.net
>>> Link: http://lkml.kernel.org/r/20120319231253.GA20...@www.outflux.net
>>> Signed-off-by: Thomas Gleixner <t...@linutronix.de>
>>> ---
>>>  kernel/futex.c        |   36 +++++++++++++-----------------------
>>>  kernel/futex_compat.c |   36 +++++++++++++-----------------------
>>>  2 files changed, 26 insertions(+), 46 deletions(-)

>>> diff --git a/kernel/futex.c b/kernel/futex.c
>>> index 72efa1e..d701be5 100644
>>> --- a/kernel/futex.c
>>> +++ b/kernel/futex.c
>>> @@ -59,6 +59,7 @@
>>>  #include <linux/magic.h>
>>>  #include <linux/pid.h>
>>>  #include <linux/nsproxy.h>
>>> +#include <linux/ptrace.h>

>>>  #include <asm/futex.h>

>>> @@ -2443,40 +2444,29 @@ SYSCALL_DEFINE3(get_robust_list, int, pid,
>>>  {
>>>    struct robust_list_head __user *head;
>>>    unsigned long ret;
>>> -  const struct cred *cred = current_cred(), *pcred;
>>> +  struct task_struct *p;

>>>    if (!futex_cmpxchg_enabled)
>>>            return -ENOSYS;

>>> +  rcu_read_lock();
>>> +
>>> +  ret = -ESRCH;
>>>    if (!pid)
>>> -          head = current->robust_list;
>>> +          p = current;
>>>    else {
>>> -          struct task_struct *p;
>>> -
>>> -          ret = -ESRCH;
>>> -          rcu_read_lock();
>>>            p = find_task_by_vpid(pid);
>>>            if (!p)
>>>                    goto err_unlock;
>>> -          ret = -EPERM;
>>> -          pcred = __task_cred(p);
>>> -          /* If victim is in different user_ns, then uids are not
>>> -             comparable, so we must have CAP_SYS_PTRACE */
>>> -          if (cred->user->user_ns != pcred->user->user_ns) {
>>> -                  if (!ns_capable(pcred->user->user_ns, CAP_SYS_PTRACE))
>>> -                          goto err_unlock;
>>> -                  goto ok;
>>> -          }
>>> -          /* If victim is in same user_ns, then uids are comparable */
>>> -          if (cred->euid != pcred->euid &&
>>> -              cred->euid != pcred->uid &&
>>> -              !ns_capable(pcred->user->user_ns, CAP_SYS_PTRACE))
>>> -                  goto err_unlock;
>>> -ok:
>>> -          head = p->robust_list;
>>> -          rcu_read_unlock();
>>>    }

>>> +  ret = -EPERM;
>>> +  if (!ptrace_may_access(p, PTRACE_MODE_READ))
>>> +          goto err_unlock;
>>> +
>>> +  head = p->robust_list;
>>> +  rcu_read_unlock();
>>> +
>>>    if (put_user(sizeof(*head), len_ptr))
>>>            return -EFAULT;
>>>    return put_user(head, head_ptr);
>>> diff --git a/kernel/futex_compat.c b/kernel/futex_compat.c
>>> index 5f9e689..a9642d5 100644
>>> --- a/kernel/futex_compat.c
>>> +++ b/kernel/futex_compat.c
>>> @@ -10,6 +10,7 @@
>>>  #include <linux/compat.h>
>>>  #include <linux/nsproxy.h>
>>>  #include <linux/futex.h>
>>> +#include <linux/ptrace.h>

>>>  #include <asm/uaccess.h>

>>> @@ -136,40 +137,29 @@ compat_sys_get_robust_list(int pid, compat_uptr_t __user *head_ptr,
>>>  {
>>>    struct compat_robust_list_head __user *head;
>>>    unsigned long ret;
>>> -  const struct cred *cred = current_cred(), *pcred;
>>> +  struct task_struct *p;

>>>    if (!futex_cmpxchg_enabled)
>>>            return -ENOSYS;

>>> +  rcu_read_lock();
>>> +
>>> +  ret = -ESRCH;
>>>    if (!pid)
>>> -          head = current->compat_robust_list;
>>> +          p = current;
>>>    else {
>>> -          struct task_struct *p;
>>> -
>>> -          ret = -ESRCH;
>>> -          rcu_read_lock();
>>>            p = find_task_by_vpid(pid);
>>>            if (!p)
>>>                    goto err_unlock;
>>> -          ret = -EPERM;
>>> -          pcred = __task_cred(p);
>>> -          /* If victim is in different user_ns, then uids are not
>>> -             comparable, so we must have CAP_SYS_PTRACE */
>>> -          if (cred->user->user_ns != pcred->user->user_ns) {
>>> -                  if (!ns_capable(pcred->user->user_ns, CAP_SYS_PTRACE))
>>> -                          goto err_unlock;
>>> -                  goto ok;
>>> -          }
>>> -          /* If victim is in same user_ns, then uids are comparable */
>>> -          if (cred->euid != pcred->euid &&
>>> -              cred->euid != pcred->uid &&
>>> -              !ns_capable(pcred->user->user_ns, CAP_SYS_PTRACE))
>>> -                  goto err_unlock;
>>> -ok:
>>> -          head = p->compat_robust_list;
>>> -          rcu_read_unlock();
>>>    }

>>> +  ret = -EPERM;
>>> +  if (!ptrace_may_access(p, PTRACE_MODE_READ))
>>> +          goto err_unlock;
>>> +
>>> +  head = p->compat_robust_list;
>>> +  rcu_read_unlock();
>>> +
>>>    if (put_user(sizeof(*head), len_ptr))
>>>            return -EFAULT;
>>>    return put_user(ptr_to_compat(head), head_ptr);
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at  http://www.tux.org/lkml/

> >>> futex: Do not leak robust list to unprivileged process

> >>> It was possible to extract the robust list head address from a setuid
> >>> process if it had used set_robust_list(), allowing an ASLR info leak. This
> >>> changes the permission checks to be the same as those used for similar
> >>> info that comes out of /proc.

> >>> Running a setuid program that uses robust futexes would have had:
> >>>   cred->euid != pcred->euid
> >>>   cred->euid == pcred->uid
> >>> so the old permissions check would allow it. I'm not aware of any setuid
> >>> programs that use robust futexes, so this is just a preventative measure.

> >> I'm not sure this change prevents the unprivileged process.
> >> Please refer to LTP test, recently I saw that this change broke
> >> the following test.

> >> https://github.com/linux-test-project/ltp/blob/master/testcases/kerne...
> >>           if (seteuid(1) == -1)
> >>                   tst_brkm(TBROK|TERRNO, cleanup, "seteuid(1) failed");

> >>           TEST(retval = syscall(__NR_get_robust_list, 1,
> >>                                 (struct robust_list_head *)&head,
> >>                                 &len_ptr));

> >> We set the euid to an unprivileged user, and expect to FAIL with EPERM,
> >> without this patch, it FAIL as we expected, but with it, this call succeed.

> > This relates to a question I asked - I believe in this thread, maybe in
> > another thread - about ptrace_may_access.  That code goes back further than
> > our git history, and for so long has used current->uid and ->gid, not
> > euid and gid, for permission checks.  I asked if that's what we really
> > want, but at the same am not sure we want to change something that's
> > been like that for so long.

> > But that's why it succeeded - you changed your euid, not your uid.

> Yeah, I known what I'm doing.