* [PATCH] futex: do not leak robust list to unprivileged process @ 2012-03-19 23:12 ` Kees Cook 0 siblings, 0 replies; 78+ messages in thread From: Kees Cook @ 2012-03-19 23:12 UTC (permalink / raw) To: linux-kernel Cc: Darren Hart, Thomas Gleixner, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, Serge E. Hallyn, kernel-hardening, spender 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 <keescook@chromium.org> --- 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 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 ^ permalink raw reply related [flat|nested] 78+ messages in thread
* [kernel-hardening] [PATCH] futex: do not leak robust list to unprivileged process @ 2012-03-19 23:12 ` Kees Cook 0 siblings, 0 replies; 78+ messages in thread From: Kees Cook @ 2012-03-19 23:12 UTC (permalink / raw) To: linux-kernel Cc: Darren Hart, Thomas Gleixner, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, Serge E. Hallyn, kernel-hardening, spender 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 <keescook@chromium.org> --- 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 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 ^ permalink raw reply related [flat|nested] 78+ messages in thread
* Re: [PATCH] futex: do not leak robust list to unprivileged process 2012-03-19 23:12 ` [kernel-hardening] " Kees Cook @ 2012-03-20 13:31 ` Serge Hallyn -1 siblings, 0 replies; 78+ messages in thread From: Serge Hallyn @ 2012-03-20 13:31 UTC (permalink / raw) To: Kees Cook Cc: linux-kernel, Darren Hart, Thomas Gleixner, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, kernel-hardening, spender, mingo Quoting Kees Cook (keescook@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 <keescook@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) Acked-by: Serge Hallyn <serge.hallyn@canonical.com> > --- > 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 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 > ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH] futex: do not leak robust list to unprivileged process @ 2012-03-20 13:31 ` Serge Hallyn 0 siblings, 0 replies; 78+ messages in thread From: Serge Hallyn @ 2012-03-20 13:31 UTC (permalink / raw) To: Kees Cook Cc: linux-kernel, Darren Hart, Thomas Gleixner, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, kernel-hardening, spender, mingo Quoting Kees Cook (keescook@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 <keescook@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) Acked-by: Serge Hallyn <serge.hallyn@canonical.com> > --- > 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 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 > ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH] futex: do not leak robust list to unprivileged process 2012-03-20 13:31 ` [kernel-hardening] " Serge Hallyn @ 2012-03-20 17:02 ` Thomas Gleixner -1 siblings, 0 replies; 78+ messages in thread From: Thomas Gleixner @ 2012-03-20 17:02 UTC (permalink / raw) To: Serge Hallyn Cc: Kees Cook, linux-kernel, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, kernel-hardening, spender, mingo On Tue, 20 Mar 2012, Serge Hallyn wrote: > Quoting Kees Cook (keescook@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 <keescook@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. Thanks, tglx ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH] futex: do not leak robust list to unprivileged process @ 2012-03-20 17:02 ` Thomas Gleixner 0 siblings, 0 replies; 78+ messages in thread From: Thomas Gleixner @ 2012-03-20 17:02 UTC (permalink / raw) To: Serge Hallyn Cc: Kees Cook, linux-kernel, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, kernel-hardening, spender, mingo On Tue, 20 Mar 2012, Serge Hallyn wrote: > Quoting Kees Cook (keescook@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 <keescook@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. Thanks, tglx ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH] futex: do not leak robust list to unprivileged process 2012-03-20 17:02 ` [kernel-hardening] " Thomas Gleixner @ 2012-03-20 17:11 ` Kees Cook -1 siblings, 0 replies; 78+ messages in thread From: Kees Cook @ 2012-03-20 17:11 UTC (permalink / raw) To: Thomas Gleixner Cc: Serge Hallyn, linux-kernel, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, kernel-hardening, spender, mingo On Tue, Mar 20, 2012 at 10:02 AM, Thomas Gleixner <tglx@linutronix.de> wrote: > On Tue, 20 Mar 2012, Serge Hallyn wrote: > >> Quoting Kees Cook (keescook@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 <keescook@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. ;) -Kees -- Kees Cook ChromeOS Security ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH] futex: do not leak robust list to unprivileged process @ 2012-03-20 17:11 ` Kees Cook 0 siblings, 0 replies; 78+ messages in thread From: Kees Cook @ 2012-03-20 17:11 UTC (permalink / raw) To: Thomas Gleixner Cc: Serge Hallyn, linux-kernel, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, kernel-hardening, spender, mingo On Tue, Mar 20, 2012 at 10:02 AM, Thomas Gleixner <tglx@linutronix.de> wrote: > On Tue, 20 Mar 2012, Serge Hallyn wrote: > >> Quoting Kees Cook (keescook@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 <keescook@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. ;) -Kees -- Kees Cook ChromeOS Security ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH] futex: do not leak robust list to unprivileged process 2012-03-20 17:11 ` [kernel-hardening] " Kees Cook @ 2012-03-20 17:23 ` Ingo Molnar -1 siblings, 0 replies; 78+ messages in thread From: Ingo Molnar @ 2012-03-20 17:23 UTC (permalink / raw) To: Kees Cook Cc: Thomas Gleixner, Serge Hallyn, linux-kernel, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, kernel-hardening, spender * Kees Cook <keescook@chromium.org> wrote: > On Tue, Mar 20, 2012 at 10:02 AM, Thomas Gleixner <tglx@linutronix.de> wrote: > > On Tue, 20 Mar 2012, Serge Hallyn wrote: > > > >> Quoting Kees Cook (keescook@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 <keescook@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 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. Thanks, Ingo ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH] futex: do not leak robust list to unprivileged process @ 2012-03-20 17:23 ` Ingo Molnar 0 siblings, 0 replies; 78+ messages in thread From: Ingo Molnar @ 2012-03-20 17:23 UTC (permalink / raw) To: Kees Cook Cc: Thomas Gleixner, Serge Hallyn, linux-kernel, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, kernel-hardening, spender * Kees Cook <keescook@chromium.org> wrote: > On Tue, Mar 20, 2012 at 10:02 AM, Thomas Gleixner <tglx@linutronix.de> wrote: > > On Tue, 20 Mar 2012, Serge Hallyn wrote: > > > >> Quoting Kees Cook (keescook@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 <keescook@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 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. Thanks, Ingo ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH] futex: do not leak robust list to unprivileged process 2012-03-20 17:23 ` [kernel-hardening] " Ingo Molnar @ 2012-03-22 23:46 ` Thomas Gleixner -1 siblings, 0 replies; 78+ messages in thread From: Thomas Gleixner @ 2012-03-22 23:46 UTC (permalink / raw) To: Ingo Molnar Cc: Kees Cook, Serge Hallyn, LKML, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, kernel-hardening, spender On Tue, 20 Mar 2012, Ingo Molnar wrote: > * Kees Cook <keescook@chromium.org> wrote: > > On Tue, Mar 20, 2012 at 10:02 AM, Thomas Gleixner <tglx@linutronix.de> wrote: > > > 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 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. Thoughts ? tglx ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH] futex: do not leak robust list to unprivileged process @ 2012-03-22 23:46 ` Thomas Gleixner 0 siblings, 0 replies; 78+ messages in thread From: Thomas Gleixner @ 2012-03-22 23:46 UTC (permalink / raw) To: Ingo Molnar Cc: Kees Cook, Serge Hallyn, LKML, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, kernel-hardening, spender On Tue, 20 Mar 2012, Ingo Molnar wrote: > * Kees Cook <keescook@chromium.org> wrote: > > On Tue, Mar 20, 2012 at 10:02 AM, Thomas Gleixner <tglx@linutronix.de> wrote: > > > 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 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. Thoughts ? tglx ^ permalink raw reply [flat|nested] 78+ messages in thread
* [PATCH] futex: mark get_robust_list as deprecated 2012-03-22 23:46 ` [kernel-hardening] " Thomas Gleixner @ 2012-03-23 17:58 ` Kees Cook -1 siblings, 0 replies; 78+ messages in thread From: Kees Cook @ 2012-03-23 17:58 UTC (permalink / raw) To: Thomas Gleixner Cc: linux-kernel, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, Serge E. Hallyn, kernel-hardening, spender Notify get_robust_list users that the syscall is going away. Suggested-by: Thomas Gleixner <tglx@linutronix.de> Signed-off-by: Kees Cook <keescook@chromium.org> --- kernel/futex.c | 2 ++ kernel/futex_compat.c | 2 ++ 2 files changed, 4 insertions(+), 0 deletions(-) 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 ^ permalink raw reply related [flat|nested] 78+ messages in thread
* [kernel-hardening] [PATCH] futex: mark get_robust_list as deprecated @ 2012-03-23 17:58 ` Kees Cook 0 siblings, 0 replies; 78+ messages in thread From: Kees Cook @ 2012-03-23 17:58 UTC (permalink / raw) To: Thomas Gleixner Cc: linux-kernel, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, Serge E. Hallyn, kernel-hardening, spender Notify get_robust_list users that the syscall is going away. Suggested-by: Thomas Gleixner <tglx@linutronix.de> Signed-off-by: Kees Cook <keescook@chromium.org> --- kernel/futex.c | 2 ++ kernel/futex_compat.c | 2 ++ 2 files changed, 4 insertions(+), 0 deletions(-) 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 ^ permalink raw reply related [flat|nested] 78+ messages in thread
* Re: [PATCH] futex: mark get_robust_list as deprecated 2012-03-23 17:58 ` [kernel-hardening] " Kees Cook @ 2012-03-23 18:27 ` Thomas Gleixner -1 siblings, 0 replies; 78+ messages in thread From: Thomas Gleixner @ 2012-03-23 18:27 UTC (permalink / raw) To: Kees Cook Cc: linux-kernel, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, Serge E. Hallyn, kernel-hardening, spender On Fri, 23 Mar 2012, Kees Cook wrote: > Notify get_robust_list users that the syscall is going away. That want's an entry in Documentation/feature-removal-schedule.txt as well. That's on top of your previous one, right ? Thanks, tglx > Suggested-by: Thomas Gleixner <tglx@linutronix.de> > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > kernel/futex.c | 2 ++ > kernel/futex_compat.c | 2 ++ > 2 files changed, 4 insertions(+), 0 deletions(-) > > 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 > ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH] futex: mark get_robust_list as deprecated @ 2012-03-23 18:27 ` Thomas Gleixner 0 siblings, 0 replies; 78+ messages in thread From: Thomas Gleixner @ 2012-03-23 18:27 UTC (permalink / raw) To: Kees Cook Cc: linux-kernel, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, Serge E. Hallyn, kernel-hardening, spender On Fri, 23 Mar 2012, Kees Cook wrote: > Notify get_robust_list users that the syscall is going away. That want's an entry in Documentation/feature-removal-schedule.txt as well. That's on top of your previous one, right ? Thanks, tglx > Suggested-by: Thomas Gleixner <tglx@linutronix.de> > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > kernel/futex.c | 2 ++ > kernel/futex_compat.c | 2 ++ > 2 files changed, 4 insertions(+), 0 deletions(-) > > 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 > ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH] futex: mark get_robust_list as deprecated 2012-03-23 18:27 ` [kernel-hardening] " Thomas Gleixner @ 2012-03-23 19:08 ` Kees Cook -1 siblings, 0 replies; 78+ messages in thread From: Kees Cook @ 2012-03-23 19:08 UTC (permalink / raw) To: Thomas Gleixner Cc: linux-kernel, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, Serge E. Hallyn, kernel-hardening, spender On Fri, Mar 23, 2012 at 11:27 AM, Thomas Gleixner <tglx@linutronix.de> wrote: > On Fri, 23 Mar 2012, Kees Cook wrote: > >> Notify get_robust_list users that the syscall is going away. > > That want's an entry in Documentation/feature-removal-schedule.txt as > well. Ah! Yes, I will send a v2. > That's on top of your previous one, right ? Yup. :) -Kees -- Kees Cook ChromeOS Security ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH] futex: mark get_robust_list as deprecated @ 2012-03-23 19:08 ` Kees Cook 0 siblings, 0 replies; 78+ messages in thread From: Kees Cook @ 2012-03-23 19:08 UTC (permalink / raw) To: Thomas Gleixner Cc: linux-kernel, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, Serge E. Hallyn, kernel-hardening, spender On Fri, Mar 23, 2012 at 11:27 AM, Thomas Gleixner <tglx@linutronix.de> wrote: > On Fri, 23 Mar 2012, Kees Cook wrote: > >> Notify get_robust_list users that the syscall is going away. > > That want's an entry in Documentation/feature-removal-schedule.txt as > well. Ah! Yes, I will send a v2. > That's on top of your previous one, right ? Yup. :) -Kees -- Kees Cook ChromeOS Security ^ permalink raw reply [flat|nested] 78+ messages in thread
* [PATCH v2] futex: mark get_robust_list as deprecated 2012-03-23 18:27 ` [kernel-hardening] " Thomas Gleixner @ 2012-03-23 19:08 ` Kees Cook -1 siblings, 0 replies; 78+ messages in thread From: Kees Cook @ 2012-03-23 19:08 UTC (permalink / raw) To: Thomas Gleixner Cc: linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender Notify get_robust_list users that the syscall is going away. Suggested-by: Thomas Gleixner <tglx@linutronix.de> Signed-off-by: Kees Cook <keescook@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 <amwang@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 <keescook@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; -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 78+ messages in thread
* [kernel-hardening] [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-03-23 19:08 ` Kees Cook 0 siblings, 0 replies; 78+ messages in thread From: Kees Cook @ 2012-03-23 19:08 UTC (permalink / raw) To: Thomas Gleixner Cc: linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender Notify get_robust_list users that the syscall is going away. Suggested-by: Thomas Gleixner <tglx@linutronix.de> Signed-off-by: Kees Cook <keescook@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 <amwang@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 <keescook@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; -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 78+ messages in thread
[parent not found: <20120323190855.GA27213-0X9Bc/hWBUTk6RaD4rd5nQ@public.gmane.org>]
* Re: [PATCH v2] futex: mark get_robust_list as deprecated 2012-03-23 19:08 ` [kernel-hardening] " Kees Cook (?) @ 2012-03-23 22:06 ` Eric W. Biederman -1 siblings, 0 replies; 78+ messages in thread From: Eric W. Biederman @ 2012-03-23 22:06 UTC (permalink / raw) To: Kees Cook Cc: spender-JNS0hek0TMl4qEwOxq4T+Q, Peter Zijlstra, linux-doc-u79uwXL29TY76Z2rM5mHXA, Jiri Kosina, Darren Hart, kernel-hardening-ZwoEplunGu1jrUoiu81ncdBPR1lH4CV8, linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells, Randy Dunlap, Linux Containers, Thomas Gleixner, Andrew Morton Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> writes: > 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 <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> > Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.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 <amwang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > + > +---------------------------- > + > +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 <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.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; ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-03-23 22:06 ` Eric W. Biederman 0 siblings, 0 replies; 78+ messages in thread From: Eric W. Biederman @ 2012-03-23 22:06 UTC (permalink / raw) To: Kees Cook Cc: Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender, Linux Containers Kees Cook <keescook@chromium.org> writes: > 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 <tglx@linutronix.de> > Signed-off-by: Kees Cook <keescook@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 <amwang@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 <keescook@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; ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-03-23 22:06 ` Eric W. Biederman 0 siblings, 0 replies; 78+ messages in thread From: Eric W. Biederman @ 2012-03-23 22:06 UTC (permalink / raw) To: Kees Cook Cc: Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender, Linux Containers Kees Cook <keescook@chromium.org> writes: > 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 <tglx@linutronix.de> > Signed-off-by: Kees Cook <keescook@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 <amwang@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 <keescook@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; ^ permalink raw reply [flat|nested] 78+ messages in thread
[parent not found: <m1fwczvuph.fsf-+imSwln9KH6u2/kzUuoCbdi2O/JbrIOy@public.gmane.org>]
* Re: [PATCH v2] futex: mark get_robust_list as deprecated 2012-03-23 22:06 ` Eric W. Biederman (?) @ 2012-03-23 22:10 ` Kees Cook -1 siblings, 0 replies; 78+ messages in thread From: Kees Cook @ 2012-03-23 22:10 UTC (permalink / raw) To: Eric W. Biederman Cc: spender-JNS0hek0TMl4qEwOxq4T+Q, Peter Zijlstra, linux-doc-u79uwXL29TY76Z2rM5mHXA, Jiri Kosina, Darren Hart, kernel-hardening-ZwoEplunGu1jrUoiu81ncdBPR1lH4CV8, linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells, Randy Dunlap, Linux Containers, Thomas Gleixner, Andrew Morton On Fri, Mar 23, 2012 at 3:06 PM, Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> wrote: > Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> writes: > >> 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. Hm, that might be true. But, I guess that's why it's lucky it's a separate patch. :) The first patch is still desirable, IMO. -Kees -- Kees Cook ChromeOS Security ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-03-23 22:10 ` Kees Cook 0 siblings, 0 replies; 78+ messages in thread From: Kees Cook @ 2012-03-23 22:10 UTC (permalink / raw) To: Eric W. Biederman Cc: Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender, Linux Containers On Fri, Mar 23, 2012 at 3:06 PM, Eric W. Biederman <ebiederm@xmission.com> wrote: > Kees Cook <keescook@chromium.org> writes: > >> 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. Hm, that might be true. But, I guess that's why it's lucky it's a separate patch. :) The first patch is still desirable, IMO. -Kees -- Kees Cook ChromeOS Security ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-03-23 22:10 ` Kees Cook 0 siblings, 0 replies; 78+ messages in thread From: Kees Cook @ 2012-03-23 22:10 UTC (permalink / raw) To: Eric W. Biederman Cc: Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender, Linux Containers On Fri, Mar 23, 2012 at 3:06 PM, Eric W. Biederman <ebiederm@xmission.com> wrote: > Kees Cook <keescook@chromium.org> writes: > >> 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. Hm, that might be true. But, I guess that's why it's lucky it's a separate patch. :) The first patch is still desirable, IMO. -Kees -- Kees Cook ChromeOS Security ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH v2] futex: mark get_robust_list as deprecated 2012-03-23 22:06 ` Eric W. Biederman (?) @ 2012-03-30 5:05 ` Matt Helsley -1 siblings, 0 replies; 78+ messages in thread From: Matt Helsley @ 2012-03-30 5:05 UTC (permalink / raw) To: Eric W. Biederman Cc: Pavel Emelyanov, spender-JNS0hek0TMl4qEwOxq4T+Q, Kees Cook, linux-doc-u79uwXL29TY76Z2rM5mHXA, Jiri Kosina, Darren Hart, kernel-hardening-ZwoEplunGu1jrUoiu81ncdBPR1lH4CV8, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrew Morton, David Howells, Gene Cooperman, Randy Dunlap, Cyrill Gorcunov, Thomas Gleixner, Linux Containers, Peter Zijlstra On Fri, Mar 23, 2012 at 03:06:02PM -0700, Eric W. Biederman wrote: > Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> writes: > > > 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 <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> > > Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.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 <amwang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > > + > > +---------------------------- > > + > > +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 ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-03-30 5:05 ` Matt Helsley 0 siblings, 0 replies; 78+ messages in thread From: Matt Helsley @ 2012-03-30 5:05 UTC (permalink / raw) To: Eric W. Biederman Cc: Kees Cook, spender, Peter Zijlstra, linux-doc, Jiri Kosina, Darren Hart, kernel-hardening, linux-kernel, David Howells, Randy Dunlap, Linux Containers, Thomas Gleixner, Andrew Morton, Cyrill Gorcunov, Pavel Emelyanov, Gene Cooperman On Fri, Mar 23, 2012 at 03:06:02PM -0700, Eric W. Biederman wrote: > Kees Cook <keescook@chromium.org> writes: > > > 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 <tglx@linutronix.de> > > Signed-off-by: Kees Cook <keescook@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 <amwang@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 ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-03-30 5:05 ` Matt Helsley 0 siblings, 0 replies; 78+ messages in thread From: Matt Helsley @ 2012-03-30 5:05 UTC (permalink / raw) To: Eric W. Biederman Cc: Kees Cook, spender, Peter Zijlstra, linux-doc, Jiri Kosina, Darren Hart, kernel-hardening, linux-kernel, David Howells, Randy Dunlap, Linux Containers, Thomas Gleixner, Andrew Morton, Cyrill Gorcunov, Pavel Emelyanov, Gene Cooperman On Fri, Mar 23, 2012 at 03:06:02PM -0700, Eric W. Biederman wrote: > Kees Cook <keescook@chromium.org> writes: > > > 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 <tglx@linutronix.de> > > Signed-off-by: Kees Cook <keescook@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 <amwang@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 ^ permalink raw reply [flat|nested] 78+ messages in thread
[parent not found: <20120330050544.GA32299-52DBMbEzqgQ/wnmkkaCWp/UQ3DHhIser@public.gmane.org>]
* Re: [PATCH v2] futex: mark get_robust_list as deprecated 2012-03-30 5:05 ` Matt Helsley (?) @ 2012-03-30 6:14 ` Pavel Emelyanov -1 siblings, 0 replies; 78+ messages in thread From: Pavel Emelyanov @ 2012-03-30 6:14 UTC (permalink / raw) To: Matt Helsley Cc: spender-JNS0hek0TMl4qEwOxq4T+Q, Peter Zijlstra, linux-doc-u79uwXL29TY76Z2rM5mHXA, Jiri Kosina, Darren Hart, kernel-hardening-ZwoEplunGu1jrUoiu81ncdBPR1lH4CV8, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrew Morton, David Howells, Gene Cooperman, Randy Dunlap, Eric W. Biederman, Cyrill Gorcunov, Thomas Gleixner, Linux Containers, Kees Cook On 03/30/2012 09:05 AM, Matt Helsley wrote: > On Fri, Mar 23, 2012 at 03:06:02PM -0700, Eric W. Biederman wrote: >> Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> writes: >> >>> 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. Nope, we don't need this syscall, thanks for notifying! >> >> Eric >> >>> Suggested-by: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> >>> Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.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 <amwang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> >>> + >>> +---------------------------- >>> + >>> +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 > > . > ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-03-30 6:14 ` Pavel Emelyanov 0 siblings, 0 replies; 78+ messages in thread From: Pavel Emelyanov @ 2012-03-30 6:14 UTC (permalink / raw) To: Matt Helsley Cc: Eric W. Biederman, Kees Cook, spender, Peter Zijlstra, linux-doc, Jiri Kosina, Darren Hart, kernel-hardening, linux-kernel, David Howells, Randy Dunlap, Linux Containers, Thomas Gleixner, Andrew Morton, Cyrill Gorcunov, Gene Cooperman On 03/30/2012 09:05 AM, Matt Helsley wrote: > On Fri, Mar 23, 2012 at 03:06:02PM -0700, Eric W. Biederman wrote: >> Kees Cook <keescook@chromium.org> writes: >> >>> 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. Nope, we don't need this syscall, thanks for notifying! >> >> Eric >> >>> Suggested-by: Thomas Gleixner <tglx@linutronix.de> >>> Signed-off-by: Kees Cook <keescook@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 <amwang@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 > > . > ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-03-30 6:14 ` Pavel Emelyanov 0 siblings, 0 replies; 78+ messages in thread From: Pavel Emelyanov @ 2012-03-30 6:14 UTC (permalink / raw) To: Matt Helsley Cc: Eric W. Biederman, Kees Cook, spender, Peter Zijlstra, linux-doc, Jiri Kosina, Darren Hart, kernel-hardening, linux-kernel, David Howells, Randy Dunlap, Linux Containers, Thomas Gleixner, Andrew Morton, Cyrill Gorcunov, Gene Cooperman On 03/30/2012 09:05 AM, Matt Helsley wrote: > On Fri, Mar 23, 2012 at 03:06:02PM -0700, Eric W. Biederman wrote: >> Kees Cook <keescook@chromium.org> writes: >> >>> 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. Nope, we don't need this syscall, thanks for notifying! >> >> Eric >> >>> Suggested-by: Thomas Gleixner <tglx@linutronix.de> >>> Signed-off-by: Kees Cook <keescook@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 <amwang@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 > > . > ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH v2] futex: mark get_robust_list as deprecated [not found] ` <20120330050544.GA32299-52DBMbEzqgQ/wnmkkaCWp/UQ3DHhIser@public.gmane.org> 2012-03-30 6:14 ` Pavel Emelyanov @ 2012-03-30 22:51 ` Gene Cooperman 1 sibling, 0 replies; 78+ messages in thread From: Gene Cooperman @ 2012-03-30 22:51 UTC (permalink / raw) To: Matt Helsley Cc: Pavel Emelyanov, spender-JNS0hek0TMl4qEwOxq4T+Q, Peter Zijlstra, linux-doc-u79uwXL29TY76Z2rM5mHXA, Jiri Kosina, Darren Hart, kernel-hardening-ZwoEplunGu1jrUoiu81ncdBPR1lH4CV8, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrew Morton, David Howells, Gene Cooperman, Randy Dunlap, Eric W. Biederman, Cyrill Gorcunov, Thomas Gleixner, Linux Containers, Kees Cook Thanks for including us in the cc, Matt. We don't need the system call for DMTCP either. Also, in our DMTCP user base, we haven't had any requests to support checkpointing of user code with get_robust_list(). If a user had needed this or a similar system call, I suspect our new plugin architecture would make it easy to eupport. But it's a non-issue now. Thanks, - Gene On Thu, Mar 29, 2012 at 10:05:44PM -0700, Matt Helsley wrote: > On Fri, Mar 23, 2012 at 03:06:02PM -0700, Eric W. Biederman wrote: > > Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> writes: > > > > > 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 <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> > > > Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.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 <amwang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > > > + > > > +---------------------------- > > > + > > > +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 > ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH v2] futex: mark get_robust_list as deprecated 2012-03-30 5:05 ` Matt Helsley @ 2012-03-30 22:51 ` Gene Cooperman -1 siblings, 0 replies; 78+ messages in thread From: Gene Cooperman @ 2012-03-30 22:51 UTC (permalink / raw) To: Matt Helsley Cc: Eric W. Biederman, Kees Cook, spender, Peter Zijlstra, linux-doc, Jiri Kosina, Darren Hart, kernel-hardening, linux-kernel, David Howells, Randy Dunlap, Linux Containers, Thomas Gleixner, Andrew Morton, Cyrill Gorcunov, Pavel Emelyanov, Gene Cooperman Thanks for including us in the cc, Matt. We don't need the system call for DMTCP either. Also, in our DMTCP user base, we haven't had any requests to support checkpointing of user code with get_robust_list(). If a user had needed this or a similar system call, I suspect our new plugin architecture would make it easy to eupport. But it's a non-issue now. Thanks, - Gene On Thu, Mar 29, 2012 at 10:05:44PM -0700, Matt Helsley wrote: > On Fri, Mar 23, 2012 at 03:06:02PM -0700, Eric W. Biederman wrote: > > Kees Cook <keescook@chromium.org> writes: > > > > > 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 <tglx@linutronix.de> > > > Signed-off-by: Kees Cook <keescook@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 <amwang@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 > ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-03-30 22:51 ` Gene Cooperman 0 siblings, 0 replies; 78+ messages in thread From: Gene Cooperman @ 2012-03-30 22:51 UTC (permalink / raw) To: Matt Helsley Cc: Eric W. Biederman, Kees Cook, spender, Peter Zijlstra, linux-doc, Jiri Kosina, Darren Hart, kernel-hardening, linux-kernel, David Howells, Randy Dunlap, Linux Containers, Thomas Gleixner, Andrew Morton, Cyrill Gorcunov, Pavel Emelyanov, Gene Cooperman Thanks for including us in the cc, Matt. We don't need the system call for DMTCP either. Also, in our DMTCP user base, we haven't had any requests to support checkpointing of user code with get_robust_list(). If a user had needed this or a similar system call, I suspect our new plugin architecture would make it easy to eupport. But it's a non-issue now. Thanks, - Gene On Thu, Mar 29, 2012 at 10:05:44PM -0700, Matt Helsley wrote: > On Fri, Mar 23, 2012 at 03:06:02PM -0700, Eric W. Biederman wrote: > > Kees Cook <keescook@chromium.org> writes: > > > > > 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 <tglx@linutronix.de> > > > Signed-off-by: Kees Cook <keescook@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 <amwang@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 > ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH v2] futex: mark get_robust_list as deprecated 2012-03-23 19:08 ` [kernel-hardening] " Kees Cook @ 2012-03-27 18:05 ` Josh Boyer -1 siblings, 0 replies; 78+ messages in thread From: Josh Boyer @ 2012-03-27 18:05 UTC (permalink / raw) To: Kees Cook Cc: Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender On Fri, Mar 23, 2012 at 3:08 PM, Kees Cook <keescook@chromium.org> wrote: > Notify get_robust_list users that the syscall is going away. > > Suggested-by: Thomas Gleixner <tglx@linutronix.de> > Signed-off-by: Kees Cook <keescook@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 <amwang@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 <keescook@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. There's nothing that can be done with those bugs other than to wait until this is removed. Maybe it won't trigger because nobody is using it, but ugh. Is printk_once sufficient? josh ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-03-27 18:05 ` Josh Boyer 0 siblings, 0 replies; 78+ messages in thread From: Josh Boyer @ 2012-03-27 18:05 UTC (permalink / raw) To: Kees Cook Cc: Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender On Fri, Mar 23, 2012 at 3:08 PM, Kees Cook <keescook@chromium.org> wrote: > Notify get_robust_list users that the syscall is going away. > > Suggested-by: Thomas Gleixner <tglx@linutronix.de> > Signed-off-by: Kees Cook <keescook@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 <amwang@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 <keescook@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. There's nothing that can be done with those bugs other than to wait until this is removed. Maybe it won't trigger because nobody is using it, but ugh. Is printk_once sufficient? josh ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH v2] futex: mark get_robust_list as deprecated 2012-03-27 18:05 ` [kernel-hardening] " Josh Boyer @ 2012-03-27 19:13 ` Peter Zijlstra -1 siblings, 0 replies; 78+ messages in thread From: Peter Zijlstra @ 2012-03-27 19:13 UTC (permalink / raw) To: Josh Boyer Cc: Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender On Tue, 2012-03-27 at 14:05 -0400, Josh Boyer wrote: > > 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. That's a positive, right? That gives extra visibility to who and what might possibly use this thing. ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-03-27 19:13 ` Peter Zijlstra 0 siblings, 0 replies; 78+ messages in thread From: Peter Zijlstra @ 2012-03-27 19:13 UTC (permalink / raw) To: Josh Boyer Cc: Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender On Tue, 2012-03-27 at 14:05 -0400, Josh Boyer wrote: > > 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. That's a positive, right? That gives extra visibility to who and what might possibly use this thing. ^ permalink raw reply [flat|nested] 78+ messages in thread
* [tip:core/locking] futex: Mark get_robust_list as deprecated 2012-03-23 19:08 ` [kernel-hardening] " Kees Cook ` (2 preceding siblings ...) (?) @ 2012-03-29 9:56 ` tip-bot for Kees Cook -1 siblings, 0 replies; 78+ messages in thread From: tip-bot for Kees Cook @ 2012-03-29 9:56 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, hpa, mingo, serge.hallyn, a.p.zijlstra, dvhart, jkosina, rdunlap, ebiederm, dhowells, keescook, tglx Commit-ID: ec0c4274e33c0373e476b73e01995c53128f1257 Gitweb: http://git.kernel.org/tip/ec0c4274e33c0373e476b73e01995c53128f1257 Author: Kees Cook <keescook@chromium.org> AuthorDate: Fri, 23 Mar 2012 12:08:55 -0700 Committer: Thomas Gleixner <tglx@linutronix.de> CommitDate: Thu, 29 Mar 2012 11:37:17 +0200 futex: Mark get_robust_list as deprecated Notify get_robust_list users that the syscall is going away. Suggested-by: Thomas Gleixner <tglx@linutronix.de> Signed-off-by: Kees Cook <keescook@chromium.org> Cc: Randy Dunlap <rdunlap@xenotime.net> Cc: Darren Hart <dvhart@linux.intel.com> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> Cc: Jiri Kosina <jkosina@suse.cz> Cc: Eric W. Biederman <ebiederm@xmission.com> Cc: David Howells <dhowells@redhat.com> Cc: Serge E. Hallyn <serge.hallyn@canonical.com> Cc: kernel-hardening@lists.openwall.com Cc: spender@grsecurity.net Link: http://lkml.kernel.org/r/20120323190855.GA27213@www.outflux.net Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- 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 0cad480..c1be806 100644 --- a/Documentation/feature-removal-schedule.txt +++ b/Documentation/feature-removal-schedule.txt @@ -529,3 +529,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 <amwang@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 <keescook@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; ^ permalink raw reply related [flat|nested] 78+ messages in thread
* Re: [PATCH v2] futex: mark get_robust_list as deprecated 2012-03-23 19:08 ` [kernel-hardening] " Kees Cook @ 2012-08-02 10:35 ` richard -rw- weinberger -1 siblings, 0 replies; 78+ messages in thread From: richard -rw- weinberger @ 2012-08-02 10:35 UTC (permalink / raw) To: Kees Cook Cc: Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender On Fri, Mar 23, 2012 at 8:08 PM, Kees Cook <keescook@chromium.org> wrote: > Notify get_robust_list users that the syscall is going away. > > Suggested-by: Thomas Gleixner <tglx@linutronix.de> > Signed-off-by: Kees Cook <keescook@chromium.org> > --- I'm using this system call in an application and noticed that's marked as deprecated now. My application collects all kind of information from crashing programs. It's installed in /proc/sys/kernel/core_pattern. If program X is crashing it executes get_robust_list(X) to get the address of the robust list and reads the list from /proc/X/mem. Is there another way to get the robust list from another program (by it's pid)? -- Thanks, //richard ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-08-02 10:35 ` richard -rw- weinberger 0 siblings, 0 replies; 78+ messages in thread From: richard -rw- weinberger @ 2012-08-02 10:35 UTC (permalink / raw) To: Kees Cook Cc: Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender On Fri, Mar 23, 2012 at 8:08 PM, Kees Cook <keescook@chromium.org> wrote: > Notify get_robust_list users that the syscall is going away. > > Suggested-by: Thomas Gleixner <tglx@linutronix.de> > Signed-off-by: Kees Cook <keescook@chromium.org> > --- I'm using this system call in an application and noticed that's marked as deprecated now. My application collects all kind of information from crashing programs. It's installed in /proc/sys/kernel/core_pattern. If program X is crashing it executes get_robust_list(X) to get the address of the robust list and reads the list from /proc/X/mem. Is there another way to get the robust list from another program (by it's pid)? -- Thanks, //richard ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH v2] futex: mark get_robust_list as deprecated 2012-08-02 10:35 ` [kernel-hardening] " richard -rw- weinberger @ 2012-08-02 11:11 ` Eric W. Biederman -1 siblings, 0 replies; 78+ messages in thread From: Eric W. Biederman @ 2012-08-02 11:11 UTC (permalink / raw) To: richard -rw- weinberger Cc: Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender richard -rw- weinberger <richard.weinberger@gmail.com> writes: > On Fri, Mar 23, 2012 at 8:08 PM, Kees Cook <keescook@chromium.org> wrote: >> Notify get_robust_list users that the syscall is going away. >> >> Suggested-by: Thomas Gleixner <tglx@linutronix.de> >> Signed-off-by: Kees Cook <keescook@chromium.org> >> --- > > I'm using this system call in an application and noticed that's marked > as deprecated now. > My application collects all kind of information from crashing programs. > It's installed in /proc/sys/kernel/core_pattern. > > If program X is crashing it executes get_robust_list(X) to get the > address of the robust list > and reads the list from /proc/X/mem. > > Is there another way to get the robust list from another program (by it's pid)? The folks doing checkpoint/restart claim to not need this, so there might be a way either that or they just haven't hit this problem yet. What you are doing sounds like a reasonable use of get_robust_list to me. Eric ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-08-02 11:11 ` Eric W. Biederman 0 siblings, 0 replies; 78+ messages in thread From: Eric W. Biederman @ 2012-08-02 11:11 UTC (permalink / raw) To: richard -rw- weinberger Cc: Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender richard -rw- weinberger <richard.weinberger@gmail.com> writes: > On Fri, Mar 23, 2012 at 8:08 PM, Kees Cook <keescook@chromium.org> wrote: >> Notify get_robust_list users that the syscall is going away. >> >> Suggested-by: Thomas Gleixner <tglx@linutronix.de> >> Signed-off-by: Kees Cook <keescook@chromium.org> >> --- > > I'm using this system call in an application and noticed that's marked > as deprecated now. > My application collects all kind of information from crashing programs. > It's installed in /proc/sys/kernel/core_pattern. > > If program X is crashing it executes get_robust_list(X) to get the > address of the robust list > and reads the list from /proc/X/mem. > > Is there another way to get the robust list from another program (by it's pid)? The folks doing checkpoint/restart claim to not need this, so there might be a way either that or they just haven't hit this problem yet. What you are doing sounds like a reasonable use of get_robust_list to me. Eric ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH v2] futex: mark get_robust_list as deprecated 2012-08-02 11:11 ` [kernel-hardening] " Eric W. Biederman @ 2012-08-03 10:17 ` richard -rw- weinberger -1 siblings, 0 replies; 78+ messages in thread From: richard -rw- weinberger @ 2012-08-03 10:17 UTC (permalink / raw) To: Eric W. Biederman Cc: Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender, Pavel Emelyanov, Cyrill Gorcunov On Thu, Aug 2, 2012 at 1:11 PM, Eric W. Biederman <ebiederm@xmission.com> wrote: > richard -rw- weinberger <richard.weinberger@gmail.com> writes: > >> On Fri, Mar 23, 2012 at 8:08 PM, Kees Cook <keescook@chromium.org> wrote: >>> Notify get_robust_list users that the syscall is going away. >>> >>> Suggested-by: Thomas Gleixner <tglx@linutronix.de> >>> Signed-off-by: Kees Cook <keescook@chromium.org> >>> --- >> >> I'm using this system call in an application and noticed that's marked >> as deprecated now. >> My application collects all kind of information from crashing programs. >> It's installed in /proc/sys/kernel/core_pattern. >> >> If program X is crashing it executes get_robust_list(X) to get the >> address of the robust list >> and reads the list from /proc/X/mem. >> >> Is there another way to get the robust list from another program (by it's pid)? > > The folks doing checkpoint/restart claim to not need this, so there > might be a way either that or they just haven't hit this problem yet. > > What you are doing sounds like a reasonable use of get_robust_list to me. > CRIU folks, how do you deal with futex robust lists? -- Thanks, //richard ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-08-03 10:17 ` richard -rw- weinberger 0 siblings, 0 replies; 78+ messages in thread From: richard -rw- weinberger @ 2012-08-03 10:17 UTC (permalink / raw) To: Eric W. Biederman Cc: Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender, Pavel Emelyanov, Cyrill Gorcunov On Thu, Aug 2, 2012 at 1:11 PM, Eric W. Biederman <ebiederm@xmission.com> wrote: > richard -rw- weinberger <richard.weinberger@gmail.com> writes: > >> On Fri, Mar 23, 2012 at 8:08 PM, Kees Cook <keescook@chromium.org> wrote: >>> Notify get_robust_list users that the syscall is going away. >>> >>> Suggested-by: Thomas Gleixner <tglx@linutronix.de> >>> Signed-off-by: Kees Cook <keescook@chromium.org> >>> --- >> >> I'm using this system call in an application and noticed that's marked >> as deprecated now. >> My application collects all kind of information from crashing programs. >> It's installed in /proc/sys/kernel/core_pattern. >> >> If program X is crashing it executes get_robust_list(X) to get the >> address of the robust list >> and reads the list from /proc/X/mem. >> >> Is there another way to get the robust list from another program (by it's pid)? > > The folks doing checkpoint/restart claim to not need this, so there > might be a way either that or they just haven't hit this problem yet. > > What you are doing sounds like a reasonable use of get_robust_list to me. > CRIU folks, how do you deal with futex robust lists? -- Thanks, //richard ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH v2] futex: mark get_robust_list as deprecated 2012-08-03 10:17 ` [kernel-hardening] " richard -rw- weinberger @ 2012-08-03 11:02 ` Cyrill Gorcunov -1 siblings, 0 replies; 78+ messages in thread From: Cyrill Gorcunov @ 2012-08-03 11:02 UTC (permalink / raw) To: richard -rw- weinberger, Pavel Emelyanov Cc: Eric W. Biederman, Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender On Fri, Aug 03, 2012 at 12:17:43PM +0200, richard -rw- weinberger wrote: > On Thu, Aug 2, 2012 at 1:11 PM, Eric W. Biederman <ebiederm@xmission.com> wrote: > > richard -rw- weinberger <richard.weinberger@gmail.com> writes: > > > >> On Fri, Mar 23, 2012 at 8:08 PM, Kees Cook <keescook@chromium.org> wrote: > >>> Notify get_robust_list users that the syscall is going away. > >>> > >>> Suggested-by: Thomas Gleixner <tglx@linutronix.de> > >>> Signed-off-by: Kees Cook <keescook@chromium.org> > >>> --- > >> > >> I'm using this system call in an application and noticed that's marked > >> as deprecated now. > >> My application collects all kind of information from crashing programs. > >> It's installed in /proc/sys/kernel/core_pattern. > >> > >> If program X is crashing it executes get_robust_list(X) to get the > >> address of the robust list > >> and reads the list from /proc/X/mem. > >> > >> Is there another way to get the robust list from another program (by it's pid)? > > > > The folks doing checkpoint/restart claim to not need this, so there > > might be a way either that or they just haven't hit this problem yet. > > > > What you are doing sounds like a reasonable use of get_robust_list to me. > > > > CRIU folks, how do you deal with futex robust lists? Well, I believe we were over-optimistic in claiming that we don't need this syscall (to be fair I think we simply yet not faced the problem Eric points). So we need some way to fetch this address and set it back. If get_robust_list get deprecated maybe we could print it out in /proc/pid/stat or something? Cyrill ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-08-03 11:02 ` Cyrill Gorcunov 0 siblings, 0 replies; 78+ messages in thread From: Cyrill Gorcunov @ 2012-08-03 11:02 UTC (permalink / raw) To: richard -rw- weinberger, Pavel Emelyanov Cc: Eric W. Biederman, Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender On Fri, Aug 03, 2012 at 12:17:43PM +0200, richard -rw- weinberger wrote: > On Thu, Aug 2, 2012 at 1:11 PM, Eric W. Biederman <ebiederm@xmission.com> wrote: > > richard -rw- weinberger <richard.weinberger@gmail.com> writes: > > > >> On Fri, Mar 23, 2012 at 8:08 PM, Kees Cook <keescook@chromium.org> wrote: > >>> Notify get_robust_list users that the syscall is going away. > >>> > >>> Suggested-by: Thomas Gleixner <tglx@linutronix.de> > >>> Signed-off-by: Kees Cook <keescook@chromium.org> > >>> --- > >> > >> I'm using this system call in an application and noticed that's marked > >> as deprecated now. > >> My application collects all kind of information from crashing programs. > >> It's installed in /proc/sys/kernel/core_pattern. > >> > >> If program X is crashing it executes get_robust_list(X) to get the > >> address of the robust list > >> and reads the list from /proc/X/mem. > >> > >> Is there another way to get the robust list from another program (by it's pid)? > > > > The folks doing checkpoint/restart claim to not need this, so there > > might be a way either that or they just haven't hit this problem yet. > > > > What you are doing sounds like a reasonable use of get_robust_list to me. > > > > CRIU folks, how do you deal with futex robust lists? Well, I believe we were over-optimistic in claiming that we don't need this syscall (to be fair I think we simply yet not faced the problem Eric points). So we need some way to fetch this address and set it back. If get_robust_list get deprecated maybe we could print it out in /proc/pid/stat or something? Cyrill ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH v2] futex: mark get_robust_list as deprecated 2012-08-03 11:02 ` [kernel-hardening] " Cyrill Gorcunov @ 2012-08-03 11:19 ` richard -rw- weinberger -1 siblings, 0 replies; 78+ messages in thread From: richard -rw- weinberger @ 2012-08-03 11:19 UTC (permalink / raw) To: Cyrill Gorcunov Cc: Pavel Emelyanov, Eric W. Biederman, Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender On Fri, Aug 3, 2012 at 1:02 PM, Cyrill Gorcunov <gorcunov@openvz.org> wrote: >> >> I'm using this system call in an application and noticed that's marked >> >> as deprecated now. >> >> My application collects all kind of information from crashing programs. >> >> It's installed in /proc/sys/kernel/core_pattern. >> >> >> >> If program X is crashing it executes get_robust_list(X) to get the >> >> address of the robust list >> >> and reads the list from /proc/X/mem. >> >> >> >> Is there another way to get the robust list from another program (by it's pid)? >> > >> > The folks doing checkpoint/restart claim to not need this, so there >> > might be a way either that or they just haven't hit this problem yet. >> > >> > What you are doing sounds like a reasonable use of get_robust_list to me. >> > >> >> CRIU folks, how do you deal with futex robust lists? > > Well, I believe we were over-optimistic in claiming that we don't need this > syscall (to be fair I think we simply yet not faced the problem Eric points). > So we need some way to fetch this address and set it back. If get_robust_list > get deprecated maybe we could print it out in /proc/pid/stat or something? Kees, you said get_robust_list() can be used to bypass ASLR. How? What makes it worse than /proc/pid/maps? If the robust list address itself is bad, removing get_robust_list() and putting the information into /proc is useless. -- Thanks, //richard ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-08-03 11:19 ` richard -rw- weinberger 0 siblings, 0 replies; 78+ messages in thread From: richard -rw- weinberger @ 2012-08-03 11:19 UTC (permalink / raw) To: Cyrill Gorcunov Cc: Pavel Emelyanov, Eric W. Biederman, Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender On Fri, Aug 3, 2012 at 1:02 PM, Cyrill Gorcunov <gorcunov@openvz.org> wrote: >> >> I'm using this system call in an application and noticed that's marked >> >> as deprecated now. >> >> My application collects all kind of information from crashing programs. >> >> It's installed in /proc/sys/kernel/core_pattern. >> >> >> >> If program X is crashing it executes get_robust_list(X) to get the >> >> address of the robust list >> >> and reads the list from /proc/X/mem. >> >> >> >> Is there another way to get the robust list from another program (by it's pid)? >> > >> > The folks doing checkpoint/restart claim to not need this, so there >> > might be a way either that or they just haven't hit this problem yet. >> > >> > What you are doing sounds like a reasonable use of get_robust_list to me. >> > >> >> CRIU folks, how do you deal with futex robust lists? > > Well, I believe we were over-optimistic in claiming that we don't need this > syscall (to be fair I think we simply yet not faced the problem Eric points). > So we need some way to fetch this address and set it back. If get_robust_list > get deprecated maybe we could print it out in /proc/pid/stat or something? Kees, you said get_robust_list() can be used to bypass ASLR. How? What makes it worse than /proc/pid/maps? If the robust list address itself is bad, removing get_robust_list() and putting the information into /proc is useless. -- Thanks, //richard ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH v2] futex: mark get_robust_list as deprecated 2012-08-03 11:19 ` [kernel-hardening] " richard -rw- weinberger @ 2012-08-03 11:27 ` Cyrill Gorcunov -1 siblings, 0 replies; 78+ messages in thread From: Cyrill Gorcunov @ 2012-08-03 11:27 UTC (permalink / raw) To: richard -rw- weinberger Cc: Pavel Emelyanov, Eric W. Biederman, Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender On Fri, Aug 03, 2012 at 01:19:24PM +0200, richard -rw- weinberger wrote: > >> > >> CRIU folks, how do you deal with futex robust lists? > > > > Well, I believe we were over-optimistic in claiming that we don't need this > > syscall (to be fair I think we simply yet not faced the problem Eric points). > > So we need some way to fetch this address and set it back. If get_robust_list > > get deprecated maybe we could print it out in /proc/pid/stat or something? > > Kees, you said get_robust_list() can be used to bypass ASLR. > How? What makes it worse than /proc/pid/maps? > > If the robust list address itself is bad, removing get_robust_list() > and putting the information into /proc is useless. Look, the /proc entry might check for some CAP and do not allow a regular user to fetch this address. Cyrill ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-08-03 11:27 ` Cyrill Gorcunov 0 siblings, 0 replies; 78+ messages in thread From: Cyrill Gorcunov @ 2012-08-03 11:27 UTC (permalink / raw) To: richard -rw- weinberger Cc: Pavel Emelyanov, Eric W. Biederman, Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender On Fri, Aug 03, 2012 at 01:19:24PM +0200, richard -rw- weinberger wrote: > >> > >> CRIU folks, how do you deal with futex robust lists? > > > > Well, I believe we were over-optimistic in claiming that we don't need this > > syscall (to be fair I think we simply yet not faced the problem Eric points). > > So we need some way to fetch this address and set it back. If get_robust_list > > get deprecated maybe we could print it out in /proc/pid/stat or something? > > Kees, you said get_robust_list() can be used to bypass ASLR. > How? What makes it worse than /proc/pid/maps? > > If the robust list address itself is bad, removing get_robust_list() > and putting the information into /proc is useless. Look, the /proc entry might check for some CAP and do not allow a regular user to fetch this address. Cyrill ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH v2] futex: mark get_robust_list as deprecated 2012-08-03 11:27 ` [kernel-hardening] " Cyrill Gorcunov @ 2012-08-03 11:30 ` richard -rw- weinberger -1 siblings, 0 replies; 78+ messages in thread From: richard -rw- weinberger @ 2012-08-03 11:30 UTC (permalink / raw) To: Cyrill Gorcunov Cc: Pavel Emelyanov, Eric W. Biederman, Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender On Fri, Aug 3, 2012 at 1:27 PM, Cyrill Gorcunov <gorcunov@openvz.org> wrote: > On Fri, Aug 03, 2012 at 01:19:24PM +0200, richard -rw- weinberger wrote: >> >> >> >> CRIU folks, how do you deal with futex robust lists? >> > >> > Well, I believe we were over-optimistic in claiming that we don't need this >> > syscall (to be fair I think we simply yet not faced the problem Eric points). >> > So we need some way to fetch this address and set it back. If get_robust_list >> > get deprecated maybe we could print it out in /proc/pid/stat or something? >> >> Kees, you said get_robust_list() can be used to bypass ASLR. >> How? What makes it worse than /proc/pid/maps? >> >> If the robust list address itself is bad, removing get_robust_list() >> and putting the information into /proc is useless. > > Look, the /proc entry might check for some CAP and do not allow > a regular user to fetch this address. We could also add another check to get_robust_list(). It does already ptrace_may_access(). -- Thanks, //richard ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-08-03 11:30 ` richard -rw- weinberger 0 siblings, 0 replies; 78+ messages in thread From: richard -rw- weinberger @ 2012-08-03 11:30 UTC (permalink / raw) To: Cyrill Gorcunov Cc: Pavel Emelyanov, Eric W. Biederman, Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender On Fri, Aug 3, 2012 at 1:27 PM, Cyrill Gorcunov <gorcunov@openvz.org> wrote: > On Fri, Aug 03, 2012 at 01:19:24PM +0200, richard -rw- weinberger wrote: >> >> >> >> CRIU folks, how do you deal with futex robust lists? >> > >> > Well, I believe we were over-optimistic in claiming that we don't need this >> > syscall (to be fair I think we simply yet not faced the problem Eric points). >> > So we need some way to fetch this address and set it back. If get_robust_list >> > get deprecated maybe we could print it out in /proc/pid/stat or something? >> >> Kees, you said get_robust_list() can be used to bypass ASLR. >> How? What makes it worse than /proc/pid/maps? >> >> If the robust list address itself is bad, removing get_robust_list() >> and putting the information into /proc is useless. > > Look, the /proc entry might check for some CAP and do not allow > a regular user to fetch this address. We could also add another check to get_robust_list(). It does already ptrace_may_access(). -- Thanks, //richard ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH v2] futex: mark get_robust_list as deprecated 2012-08-03 11:30 ` [kernel-hardening] " richard -rw- weinberger @ 2012-08-03 11:35 ` Cyrill Gorcunov -1 siblings, 0 replies; 78+ messages in thread From: Cyrill Gorcunov @ 2012-08-03 11:35 UTC (permalink / raw) To: richard -rw- weinberger Cc: Pavel Emelyanov, Eric W. Biederman, Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender On Fri, Aug 03, 2012 at 01:30:31PM +0200, richard -rw- weinberger wrote: > On Fri, Aug 3, 2012 at 1:27 PM, Cyrill Gorcunov <gorcunov@openvz.org> wrote: > > On Fri, Aug 03, 2012 at 01:19:24PM +0200, richard -rw- weinberger wrote: > >> >> > >> >> CRIU folks, how do you deal with futex robust lists? > >> > > >> > Well, I believe we were over-optimistic in claiming that we don't need this > >> > syscall (to be fair I think we simply yet not faced the problem Eric points). > >> > So we need some way to fetch this address and set it back. If get_robust_list > >> > get deprecated maybe we could print it out in /proc/pid/stat or something? > >> > >> Kees, you said get_robust_list() can be used to bypass ASLR. > >> How? What makes it worse than /proc/pid/maps? > >> > >> If the robust list address itself is bad, removing get_robust_list() > >> and putting the information into /proc is useless. > > > > Look, the /proc entry might check for some CAP and do not allow > > a regular user to fetch this address. > > We could also add another check to get_robust_list(). > It does already ptrace_may_access(). Yes, and I'm definitely not against that ;) The problem is that this syscall was marked as deprecated and if people want to drop it we need to find a way to provide this address back in a sake of c/r. If c/r is the only _one_ who needs this facility than providing the address via /proc might be worth thing to do (since I can wrap it with CONFIG_CHECKPOINT_RESTORE and a regular kernel won't see this snippet at all). Cyrill ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-08-03 11:35 ` Cyrill Gorcunov 0 siblings, 0 replies; 78+ messages in thread From: Cyrill Gorcunov @ 2012-08-03 11:35 UTC (permalink / raw) To: richard -rw- weinberger Cc: Pavel Emelyanov, Eric W. Biederman, Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender On Fri, Aug 03, 2012 at 01:30:31PM +0200, richard -rw- weinberger wrote: > On Fri, Aug 3, 2012 at 1:27 PM, Cyrill Gorcunov <gorcunov@openvz.org> wrote: > > On Fri, Aug 03, 2012 at 01:19:24PM +0200, richard -rw- weinberger wrote: > >> >> > >> >> CRIU folks, how do you deal with futex robust lists? > >> > > >> > Well, I believe we were over-optimistic in claiming that we don't need this > >> > syscall (to be fair I think we simply yet not faced the problem Eric points). > >> > So we need some way to fetch this address and set it back. If get_robust_list > >> > get deprecated maybe we could print it out in /proc/pid/stat or something? > >> > >> Kees, you said get_robust_list() can be used to bypass ASLR. > >> How? What makes it worse than /proc/pid/maps? > >> > >> If the robust list address itself is bad, removing get_robust_list() > >> and putting the information into /proc is useless. > > > > Look, the /proc entry might check for some CAP and do not allow > > a regular user to fetch this address. > > We could also add another check to get_robust_list(). > It does already ptrace_may_access(). Yes, and I'm definitely not against that ;) The problem is that this syscall was marked as deprecated and if people want to drop it we need to find a way to provide this address back in a sake of c/r. If c/r is the only _one_ who needs this facility than providing the address via /proc might be worth thing to do (since I can wrap it with CONFIG_CHECKPOINT_RESTORE and a regular kernel won't see this snippet at all). Cyrill ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH v2] futex: mark get_robust_list as deprecated 2012-08-03 11:35 ` [kernel-hardening] " Cyrill Gorcunov @ 2012-08-03 11:38 ` richard -rw- weinberger -1 siblings, 0 replies; 78+ messages in thread From: richard -rw- weinberger @ 2012-08-03 11:38 UTC (permalink / raw) To: Cyrill Gorcunov Cc: Pavel Emelyanov, Eric W. Biederman, Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender On Fri, Aug 3, 2012 at 1:35 PM, Cyrill Gorcunov <gorcunov@openvz.org> wrote: > On Fri, Aug 03, 2012 at 01:30:31PM +0200, richard -rw- weinberger wrote: >> On Fri, Aug 3, 2012 at 1:27 PM, Cyrill Gorcunov <gorcunov@openvz.org> wrote: >> > On Fri, Aug 03, 2012 at 01:19:24PM +0200, richard -rw- weinberger wrote: >> >> >> >> >> >> CRIU folks, how do you deal with futex robust lists? >> >> > >> >> > Well, I believe we were over-optimistic in claiming that we don't need this >> >> > syscall (to be fair I think we simply yet not faced the problem Eric points). >> >> > So we need some way to fetch this address and set it back. If get_robust_list >> >> > get deprecated maybe we could print it out in /proc/pid/stat or something? >> >> >> >> Kees, you said get_robust_list() can be used to bypass ASLR. >> >> How? What makes it worse than /proc/pid/maps? >> >> >> >> If the robust list address itself is bad, removing get_robust_list() >> >> and putting the information into /proc is useless. >> > >> > Look, the /proc entry might check for some CAP and do not allow >> > a regular user to fetch this address. >> >> We could also add another check to get_robust_list(). >> It does already ptrace_may_access(). > > Yes, and I'm definitely not against that ;) The problem is that this > syscall was marked as deprecated and if people want to drop it we > need to find a way to provide this address back in a sake of c/r. > > If c/r is the only _one_ who needs this facility than providing the > address via /proc might be worth thing to do (since I can wrap > it with CONFIG_CHECKPOINT_RESTORE and a regular kernel won't see > this snippet at all). Please see my first mail above. c/r is not the only user. :-P -- Thanks, //richard ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-08-03 11:38 ` richard -rw- weinberger 0 siblings, 0 replies; 78+ messages in thread From: richard -rw- weinberger @ 2012-08-03 11:38 UTC (permalink / raw) To: Cyrill Gorcunov Cc: Pavel Emelyanov, Eric W. Biederman, Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender On Fri, Aug 3, 2012 at 1:35 PM, Cyrill Gorcunov <gorcunov@openvz.org> wrote: > On Fri, Aug 03, 2012 at 01:30:31PM +0200, richard -rw- weinberger wrote: >> On Fri, Aug 3, 2012 at 1:27 PM, Cyrill Gorcunov <gorcunov@openvz.org> wrote: >> > On Fri, Aug 03, 2012 at 01:19:24PM +0200, richard -rw- weinberger wrote: >> >> >> >> >> >> CRIU folks, how do you deal with futex robust lists? >> >> > >> >> > Well, I believe we were over-optimistic in claiming that we don't need this >> >> > syscall (to be fair I think we simply yet not faced the problem Eric points). >> >> > So we need some way to fetch this address and set it back. If get_robust_list >> >> > get deprecated maybe we could print it out in /proc/pid/stat or something? >> >> >> >> Kees, you said get_robust_list() can be used to bypass ASLR. >> >> How? What makes it worse than /proc/pid/maps? >> >> >> >> If the robust list address itself is bad, removing get_robust_list() >> >> and putting the information into /proc is useless. >> > >> > Look, the /proc entry might check for some CAP and do not allow >> > a regular user to fetch this address. >> >> We could also add another check to get_robust_list(). >> It does already ptrace_may_access(). > > Yes, and I'm definitely not against that ;) The problem is that this > syscall was marked as deprecated and if people want to drop it we > need to find a way to provide this address back in a sake of c/r. > > If c/r is the only _one_ who needs this facility than providing the > address via /proc might be worth thing to do (since I can wrap > it with CONFIG_CHECKPOINT_RESTORE and a regular kernel won't see > this snippet at all). Please see my first mail above. c/r is not the only user. :-P -- Thanks, //richard ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH v2] futex: mark get_robust_list as deprecated 2012-08-03 11:02 ` [kernel-hardening] " Cyrill Gorcunov @ 2012-08-03 12:38 ` Pavel Emelyanov -1 siblings, 0 replies; 78+ messages in thread From: Pavel Emelyanov @ 2012-08-03 12:38 UTC (permalink / raw) To: Cyrill Gorcunov, richard -rw- weinberger Cc: Eric W. Biederman, Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender On 08/03/2012 03:02 PM, Cyrill Gorcunov wrote: > On Fri, Aug 03, 2012 at 12:17:43PM +0200, richard -rw- weinberger wrote: >> On Thu, Aug 2, 2012 at 1:11 PM, Eric W. Biederman <ebiederm@xmission.com> wrote: >>> richard -rw- weinberger <richard.weinberger@gmail.com> writes: >>> >>>> On Fri, Mar 23, 2012 at 8:08 PM, Kees Cook <keescook@chromium.org> wrote: >>>>> Notify get_robust_list users that the syscall is going away. >>>>> >>>>> Suggested-by: Thomas Gleixner <tglx@linutronix.de> >>>>> Signed-off-by: Kees Cook <keescook@chromium.org> >>>>> --- >>>> >>>> I'm using this system call in an application and noticed that's marked >>>> as deprecated now. >>>> My application collects all kind of information from crashing programs. >>>> It's installed in /proc/sys/kernel/core_pattern. >>>> >>>> If program X is crashing it executes get_robust_list(X) to get the >>>> address of the robust list >>>> and reads the list from /proc/X/mem. >>>> >>>> Is there another way to get the robust list from another program (by it's pid)? >>> >>> The folks doing checkpoint/restart claim to not need this, so there >>> might be a way either that or they just haven't hit this problem yet. >>> >>> What you are doing sounds like a reasonable use of get_robust_list to me. >>> >> >> CRIU folks, how do you deal with futex robust lists? > > Well, I believe we were over-optimistic in claiming that we don't need this > syscall (to be fair I think we simply yet not faced the problem Eric points). > So we need some way to fetch this address and set it back. If get_robust_list > get deprecated maybe we could print it out in /proc/pid/stat or something? I agree with Cyrill, sorry for the confusion last time. We do need some way to get the list location. The exact API is not critical, we can work with either of the mentioned above. > Cyrill > . > ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-08-03 12:38 ` Pavel Emelyanov 0 siblings, 0 replies; 78+ messages in thread From: Pavel Emelyanov @ 2012-08-03 12:38 UTC (permalink / raw) To: Cyrill Gorcunov, richard -rw- weinberger Cc: Eric W. Biederman, Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender On 08/03/2012 03:02 PM, Cyrill Gorcunov wrote: > On Fri, Aug 03, 2012 at 12:17:43PM +0200, richard -rw- weinberger wrote: >> On Thu, Aug 2, 2012 at 1:11 PM, Eric W. Biederman <ebiederm@xmission.com> wrote: >>> richard -rw- weinberger <richard.weinberger@gmail.com> writes: >>> >>>> On Fri, Mar 23, 2012 at 8:08 PM, Kees Cook <keescook@chromium.org> wrote: >>>>> Notify get_robust_list users that the syscall is going away. >>>>> >>>>> Suggested-by: Thomas Gleixner <tglx@linutronix.de> >>>>> Signed-off-by: Kees Cook <keescook@chromium.org> >>>>> --- >>>> >>>> I'm using this system call in an application and noticed that's marked >>>> as deprecated now. >>>> My application collects all kind of information from crashing programs. >>>> It's installed in /proc/sys/kernel/core_pattern. >>>> >>>> If program X is crashing it executes get_robust_list(X) to get the >>>> address of the robust list >>>> and reads the list from /proc/X/mem. >>>> >>>> Is there another way to get the robust list from another program (by it's pid)? >>> >>> The folks doing checkpoint/restart claim to not need this, so there >>> might be a way either that or they just haven't hit this problem yet. >>> >>> What you are doing sounds like a reasonable use of get_robust_list to me. >>> >> >> CRIU folks, how do you deal with futex robust lists? > > Well, I believe we were over-optimistic in claiming that we don't need this > syscall (to be fair I think we simply yet not faced the problem Eric points). > So we need some way to fetch this address and set it back. If get_robust_list > get deprecated maybe we could print it out in /proc/pid/stat or something? I agree with Cyrill, sorry for the confusion last time. We do need some way to get the list location. The exact API is not critical, we can work with either of the mentioned above. > Cyrill > . > ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH v2] futex: mark get_robust_list as deprecated 2012-08-03 12:38 ` [kernel-hardening] " Pavel Emelyanov @ 2012-08-03 12:58 ` Eric W. Biederman -1 siblings, 0 replies; 78+ messages in thread From: Eric W. Biederman @ 2012-08-03 12:58 UTC (permalink / raw) To: Pavel Emelyanov, Cyrill Gorcunov, richard -rw- weinberger Cc: Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender The permissions on the syscall were fixed them withR ptrace_may_access. We have identified two legitimate use cases. It looks like it is time for someone to generate the path to remove the depreciation. Who is up for writing and testing that patch? Eric ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-08-03 12:58 ` Eric W. Biederman 0 siblings, 0 replies; 78+ messages in thread From: Eric W. Biederman @ 2012-08-03 12:58 UTC (permalink / raw) To: Pavel Emelyanov, Cyrill Gorcunov, richard -rw- weinberger Cc: Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender The permissions on the syscall were fixed them withR ptrace_may_access. We have identified two legitimate use cases. It looks like it is time for someone to generate the path to remove the depreciation. Who is up for writing and testing that patch? Eric ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH v2] futex: mark get_robust_list as deprecated 2012-08-03 12:58 ` [kernel-hardening] " Eric W. Biederman @ 2012-08-03 13:00 ` richard -rw- weinberger -1 siblings, 0 replies; 78+ messages in thread From: richard -rw- weinberger @ 2012-08-03 13:00 UTC (permalink / raw) To: Eric W. Biederman Cc: Pavel Emelyanov, Cyrill Gorcunov, Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender On Fri, Aug 3, 2012 at 2:58 PM, Eric W. Biederman <ebiederm@xmission.com> wrote: > The permissions on the syscall were fixed them withR ptrace_may_access. > > We have identified two legitimate use cases. > > It looks like it is time for someone to generate the path to remove the depreciation. > > Who is up for writing and testing that patch? I'll send a patch. -- Thanks, //richard ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-08-03 13:00 ` richard -rw- weinberger 0 siblings, 0 replies; 78+ messages in thread From: richard -rw- weinberger @ 2012-08-03 13:00 UTC (permalink / raw) To: Eric W. Biederman Cc: Pavel Emelyanov, Cyrill Gorcunov, Kees Cook, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender On Fri, Aug 3, 2012 at 2:58 PM, Eric W. Biederman <ebiederm@xmission.com> wrote: > The permissions on the syscall were fixed them withR ptrace_may_access. > > We have identified two legitimate use cases. > > It looks like it is time for someone to generate the path to remove the depreciation. > > Who is up for writing and testing that patch? I'll send a patch. -- Thanks, //richard ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH v2] futex: mark get_robust_list as deprecated 2012-08-03 12:58 ` [kernel-hardening] " Eric W. Biederman @ 2012-08-03 17:16 ` Kees Cook -1 siblings, 0 replies; 78+ messages in thread From: Kees Cook @ 2012-08-03 17:16 UTC (permalink / raw) To: Eric W. Biederman Cc: Pavel Emelyanov, Cyrill Gorcunov, richard -rw- weinberger, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender Right, I'm satisfied with the ptrace_may_access check that was added. The deprecation was suggested as an additional change. I'm okay keeping the syscall if someone actually needs it. :) -Kees On Fri, Aug 3, 2012 at 5:58 AM, Eric W. Biederman <ebiederm@xmission.com> wrote: > The permissions on the syscall were fixed them withR ptrace_may_access. > > We have identified two legitimate use cases. > > It looks like it is time for someone to generate the path to remove the depreciation. > > Who is up for writing and testing that patch? > > Eric > -- Kees Cook Chrome OS Security ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH v2] futex: mark get_robust_list as deprecated @ 2012-08-03 17:16 ` Kees Cook 0 siblings, 0 replies; 78+ messages in thread From: Kees Cook @ 2012-08-03 17:16 UTC (permalink / raw) To: Eric W. Biederman Cc: Pavel Emelyanov, Cyrill Gorcunov, richard -rw- weinberger, Thomas Gleixner, linux-kernel, Randy Dunlap, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, David Howells, Serge E. Hallyn, linux-doc, kernel-hardening, spender Right, I'm satisfied with the ptrace_may_access check that was added. The deprecation was suggested as an additional change. I'm okay keeping the syscall if someone actually needs it. :) -Kees On Fri, Aug 3, 2012 at 5:58 AM, Eric W. Biederman <ebiederm@xmission.com> wrote: > The permissions on the syscall were fixed them withR ptrace_may_access. > > We have identified two legitimate use cases. > > It looks like it is time for someone to generate the path to remove the depreciation. > > Who is up for writing and testing that patch? > > Eric > -- Kees Cook Chrome OS Security ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] [PATCH] Revert "futex: Mark get_robust_list as deprecated" 2012-08-03 17:16 ` [kernel-hardening] " Kees Cook (?) @ 2012-08-03 18:06 ` Richard Weinberger -1 siblings, 0 replies; 78+ messages in thread From: Richard Weinberger @ 2012-08-03 18:06 UTC (permalink / raw) To: linux-kernel Cc: linux-doc, dvhart, paul.gortmaker, jkosina, rob, Richard Weinberger, Randy Dunlap, Peter Zijlstra, Eric W. Biederman, David Howells, Serge E. Hallyn, kernel-hardening, spender, keescook, tglx, Cyrill Gorcunov, xemul, stable, # get_robust_list has at least two valid use cases. 1. checkpoint/restore in userspace 2. post mortem analysis Cc: Randy Dunlap <rdunlap@xenotime.net> Cc: Darren Hart <dvhart@linux.intel.com> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> Cc: Jiri Kosina <jkosina@suse.cz> Cc: Eric W. Biederman <ebiederm@xmission.com> Cc: David Howells <dhowells@redhat.com> Cc: Serge E. Hallyn <serge.hallyn@canonical.com> Cc: kernel-hardening@lists.openwall.com Cc: spender@grsecurity.net Cc: keescook@chromium.org Cc: tglx@linutronix.de Cc: Cyrill Gorcunov <gorcunov@openvz.org> Cc: xemul@parallels.com Cc: <stable@vger.kernel.org> # 3.4.x Cc: <stable@vger.kernel.org> # 3.5.x Signed-off-by: Richard Weinberger <richard@nod.at> Acked-by: Cyrill Gorcunov <gorcunov@openvz.org> Acked-by: "Eric W. Biederman" <ebiederm@xmission.com> Acked-by: Kees Cook <keescook@chromium.org> --- Documentation/feature-removal-schedule.txt | 10 ---------- kernel/futex.c | 2 -- kernel/futex_compat.c | 2 -- 3 files changed, 14 deletions(-) diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt index afaff31..a25482e 100644 --- a/Documentation/feature-removal-schedule.txt +++ b/Documentation/feature-removal-schedule.txt @@ -506,16 +506,6 @@ Who: Sebastian Andrzej Siewior <sebastian@breakpoint.cc> ---------------------------- -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 <keescook@chromium.org> - ----------------------------- - What: Removing the pn544 raw driver. When: 3.6 Why: With the introduction of the NFC HCI and SHDL kernel layers, pn544.c diff --git a/kernel/futex.c b/kernel/futex.c index e2b0fb9..d701be5 100644 --- a/kernel/futex.c +++ b/kernel/futex.c @@ -2449,8 +2449,6 @@ 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 83e368b..a9642d5 100644 --- a/kernel/futex_compat.c +++ b/kernel/futex_compat.c @@ -142,8 +142,6 @@ 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.10.4 ^ permalink raw reply related [flat|nested] 78+ messages in thread
* Re: [PATCH] futex: do not leak robust list to unprivileged process 2012-03-22 23:46 ` [kernel-hardening] " Thomas Gleixner @ 2012-03-28 18:33 ` Kees Cook -1 siblings, 0 replies; 78+ messages in thread From: Kees Cook @ 2012-03-28 18:33 UTC (permalink / raw) To: Thomas Gleixner Cc: Ingo Molnar, Serge Hallyn, LKML, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, kernel-hardening, spender On Thu, Mar 22, 2012 at 4:46 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > On Tue, 20 Mar 2012, Ingo Molnar wrote: >> * Kees Cook <keescook@chromium.org> wrote: >> > On Tue, Mar 20, 2012 at 10:02 AM, Thomas Gleixner <tglx@linutronix.de> wrote: >> > > 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 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. :) -Kees -- Kees Cook ChromeOS Security ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH] futex: do not leak robust list to unprivileged process @ 2012-03-28 18:33 ` Kees Cook 0 siblings, 0 replies; 78+ messages in thread From: Kees Cook @ 2012-03-28 18:33 UTC (permalink / raw) To: Thomas Gleixner Cc: Ingo Molnar, Serge Hallyn, LKML, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, kernel-hardening, spender On Thu, Mar 22, 2012 at 4:46 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > On Tue, 20 Mar 2012, Ingo Molnar wrote: >> * Kees Cook <keescook@chromium.org> wrote: >> > On Tue, Mar 20, 2012 at 10:02 AM, Thomas Gleixner <tglx@linutronix.de> wrote: >> > > 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 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. :) -Kees -- Kees Cook ChromeOS Security ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [PATCH] futex: do not leak robust list to unprivileged process 2012-03-28 18:33 ` [kernel-hardening] " Kees Cook @ 2012-03-28 21:24 ` Thomas Gleixner -1 siblings, 0 replies; 78+ messages in thread From: Thomas Gleixner @ 2012-03-28 21:24 UTC (permalink / raw) To: Kees Cook Cc: Ingo Molnar, Serge Hallyn, LKML, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, kernel-hardening, spender On Wed, 28 Mar 2012, Kees Cook wrote: > On Thu, Mar 22, 2012 at 4:46 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > > On Tue, 20 Mar 2012, Ingo Molnar wrote: > >> * Kees Cook <keescook@chromium.org> wrote: > >> > On Tue, Mar 20, 2012 at 10:02 AM, Thomas Gleixner <tglx@linutronix.de> wrote: > >> > > 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 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. :) It's on my list for tomorrow. ^ permalink raw reply [flat|nested] 78+ messages in thread
* [kernel-hardening] Re: [PATCH] futex: do not leak robust list to unprivileged process @ 2012-03-28 21:24 ` Thomas Gleixner 0 siblings, 0 replies; 78+ messages in thread From: Thomas Gleixner @ 2012-03-28 21:24 UTC (permalink / raw) To: Kees Cook Cc: Ingo Molnar, Serge Hallyn, LKML, Darren Hart, Peter Zijlstra, Andrew Morton, Jiri Kosina, Eric W. Biederman, David Howells, kernel-hardening, spender On Wed, 28 Mar 2012, Kees Cook wrote: > On Thu, Mar 22, 2012 at 4:46 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > > On Tue, 20 Mar 2012, Ingo Molnar wrote: > >> * Kees Cook <keescook@chromium.org> wrote: > >> > On Tue, Mar 20, 2012 at 10:02 AM, Thomas Gleixner <tglx@linutronix.de> wrote: > >> > > 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 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. :) It's on my list for tomorrow. ^ permalink raw reply [flat|nested] 78+ messages in thread
* [tip:core/locking] futex: Do not leak robust list to unprivileged process 2012-03-19 23:12 ` [kernel-hardening] " Kees Cook (?) (?) @ 2012-03-29 9:55 ` tip-bot for Kees Cook 2012-06-19 1:41 ` Wanlong Gao -1 siblings, 1 reply; 78+ messages in thread From: tip-bot for Kees Cook @ 2012-03-29 9:55 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, hpa, mingo, serge.hallyn, a.p.zijlstra, dvhart, jkosina, ebiederm, keescook, dhowells, tglx Commit-ID: bdbb776f882f5ad431aa1e694c69c1c3d6a4a5b8 Gitweb: http://git.kernel.org/tip/bdbb776f882f5ad431aa1e694c69c1c3d6a4a5b8 Author: Kees Cook <keescook@chromium.org> AuthorDate: Mon, 19 Mar 2012 16:12:53 -0700 Committer: Thomas Gleixner <tglx@linutronix.de> CommitDate: Thu, 29 Mar 2012 11:37:17 +0200 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. (This patch is based on changes from grsecurity.) Signed-off-by: Kees Cook <keescook@chromium.org> Cc: Darren Hart <dvhart@linux.intel.com> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> Cc: Jiri Kosina <jkosina@suse.cz> Cc: Eric W. Biederman <ebiederm@xmission.com> Cc: David Howells <dhowells@redhat.com> Cc: Serge E. Hallyn <serge.hallyn@canonical.com> Cc: kernel-hardening@lists.openwall.com Cc: spender@grsecurity.net Link: http://lkml.kernel.org/r/20120319231253.GA20893@www.outflux.net Signed-off-by: Thomas Gleixner <tglx@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); ^ permalink raw reply related [flat|nested] 78+ messages in thread
* Re: [tip:core/locking] futex: Do not leak robust list to unprivileged process 2012-03-29 9:55 ` [tip:core/locking] futex: Do " tip-bot for Kees Cook @ 2012-06-19 1:41 ` Wanlong Gao 2012-06-19 2:24 ` Serge Hallyn 0 siblings, 1 reply; 78+ messages in thread From: Wanlong Gao @ 2012-06-19 1:41 UTC (permalink / raw) To: mingo, hpa, linux-kernel, serge.hallyn, dvhart, a.p.zijlstra, jkosina, ebiederm, dhowells, keescook, tglx Cc: linux-tip-commits On 03/29/2012 05:55 PM, tip-bot for Kees Cook wrote: > Commit-ID: bdbb776f882f5ad431aa1e694c69c1c3d6a4a5b8 > Gitweb: http://git.kernel.org/tip/bdbb776f882f5ad431aa1e694c69c1c3d6a4a5b8 > Author: Kees Cook <keescook@chromium.org> > AuthorDate: Mon, 19 Mar 2012 16:12:53 -0700 > Committer: Thomas Gleixner <tglx@linutronix.de> > CommitDate: Thu, 29 Mar 2012 11:37:17 +0200 > > 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/kernel/syscalls/get_robust_list/get_robust_list01.c#L155 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. 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 <keescook@chromium.org> > Cc: Darren Hart <dvhart@linux.intel.com> > Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> > Cc: Jiri Kosina <jkosina@suse.cz> > Cc: Eric W. Biederman <ebiederm@xmission.com> > Cc: David Howells <dhowells@redhat.com> > Cc: Serge E. Hallyn <serge.hallyn@canonical.com> > Cc: kernel-hardening@lists.openwall.com > Cc: spender@grsecurity.net > Link: http://lkml.kernel.org/r/20120319231253.GA20893@www.outflux.net > Signed-off-by: Thomas Gleixner <tglx@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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [tip:core/locking] futex: Do not leak robust list to unprivileged process 2012-06-19 1:41 ` Wanlong Gao @ 2012-06-19 2:24 ` Serge Hallyn 2012-06-19 2:32 ` Wanlong Gao 0 siblings, 1 reply; 78+ messages in thread From: Serge Hallyn @ 2012-06-19 2:24 UTC (permalink / raw) To: Wanlong Gao Cc: mingo, hpa, linux-kernel, dvhart, a.p.zijlstra, jkosina, ebiederm, dhowells, keescook, tglx, linux-tip-commits Quoting Wanlong Gao (gaowanlong@cn.fujitsu.com): > On 03/29/2012 05:55 PM, tip-bot for Kees Cook wrote: > > Commit-ID: bdbb776f882f5ad431aa1e694c69c1c3d6a4a5b8 > > Gitweb: http://git.kernel.org/tip/bdbb776f882f5ad431aa1e694c69c1c3d6a4a5b8 > > Author: Kees Cook <keescook@chromium.org> > > AuthorDate: Mon, 19 Mar 2012 16:12:53 -0700 > > Committer: Thomas Gleixner <tglx@linutronix.de> > > CommitDate: Thu, 29 Mar 2012 11:37:17 +0200 > > > > 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/kernel/syscalls/get_robust_list/get_robust_list01.c#L155 > 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 <keescook@chromium.org> > > Cc: Darren Hart <dvhart@linux.intel.com> > > Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> > > Cc: Jiri Kosina <jkosina@suse.cz> > > Cc: Eric W. Biederman <ebiederm@xmission.com> > > Cc: David Howells <dhowells@redhat.com> > > Cc: Serge E. Hallyn <serge.hallyn@canonical.com> > > Cc: kernel-hardening@lists.openwall.com > > Cc: spender@grsecurity.net > > Link: http://lkml.kernel.org/r/20120319231253.GA20893@www.outflux.net > > Signed-off-by: Thomas Gleixner <tglx@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 majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/ > > > > ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [tip:core/locking] futex: Do not leak robust list to unprivileged process 2012-06-19 2:24 ` Serge Hallyn @ 2012-06-19 2:32 ` Wanlong Gao 2012-06-19 3:13 ` Serge Hallyn 0 siblings, 1 reply; 78+ messages in thread From: Wanlong Gao @ 2012-06-19 2:32 UTC (permalink / raw) To: Serge Hallyn Cc: mingo, hpa, linux-kernel, dvhart, a.p.zijlstra, jkosina, ebiederm, dhowells, keescook, tglx, linux-tip-commits On 06/19/2012 10:24 AM, Serge Hallyn wrote: > Quoting Wanlong Gao (gaowanlong@cn.fujitsu.com): >> On 03/29/2012 05:55 PM, tip-bot for Kees Cook wrote: >>> Commit-ID: bdbb776f882f5ad431aa1e694c69c1c3d6a4a5b8 >>> Gitweb: http://git.kernel.org/tip/bdbb776f882f5ad431aa1e694c69c1c3d6a4a5b8 >>> Author: Kees Cook <keescook@chromium.org> >>> AuthorDate: Mon, 19 Mar 2012 16:12:53 -0700 >>> Committer: Thomas Gleixner <tglx@linutronix.de> >>> CommitDate: Thu, 29 Mar 2012 11:37:17 +0200 >>> >>> 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/kernel/syscalls/get_robust_list/get_robust_list01.c#L155 >> 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. I just wonder which is the right thing. Should we check euid or uid ? You mean that checking uid instead of checking euid for a long time, right? Thanks, Wanlong Gao > >> 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 <keescook@chromium.org> >>> Cc: Darren Hart <dvhart@linux.intel.com> >>> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> >>> Cc: Jiri Kosina <jkosina@suse.cz> >>> Cc: Eric W. Biederman <ebiederm@xmission.com> >>> Cc: David Howells <dhowells@redhat.com> >>> Cc: Serge E. Hallyn <serge.hallyn@canonical.com> >>> Cc: kernel-hardening@lists.openwall.com >>> Cc: spender@grsecurity.net >>> Link: http://lkml.kernel.org/r/20120319231253.GA20893@www.outflux.net >>> Signed-off-by: Thomas Gleixner <tglx@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 majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> Please read the FAQ at http://www.tux.org/lkml/ >>> >> >> > ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [tip:core/locking] futex: Do not leak robust list to unprivileged process 2012-06-19 2:32 ` Wanlong Gao @ 2012-06-19 3:13 ` Serge Hallyn 2012-06-19 3:21 ` Wanlong Gao 0 siblings, 1 reply; 78+ messages in thread From: Serge Hallyn @ 2012-06-19 3:13 UTC (permalink / raw) To: Wanlong Gao Cc: mingo, hpa, linux-kernel, dvhart, a.p.zijlstra, jkosina, ebiederm, dhowells, keescook, tglx, linux-tip-commits Quoting Wanlong Gao (gaowanlong@cn.fujitsu.com): > On 06/19/2012 10:24 AM, Serge Hallyn wrote: > > Quoting Wanlong Gao (gaowanlong@cn.fujitsu.com): > >> On 03/29/2012 05:55 PM, tip-bot for Kees Cook wrote: > >>> Commit-ID: bdbb776f882f5ad431aa1e694c69c1c3d6a4a5b8 > >>> Gitweb: http://git.kernel.org/tip/bdbb776f882f5ad431aa1e694c69c1c3d6a4a5b8 > >>> Author: Kees Cook <keescook@chromium.org> > >>> AuthorDate: Mon, 19 Mar 2012 16:12:53 -0700 > >>> Committer: Thomas Gleixner <tglx@linutronix.de> > >>> CommitDate: Thu, 29 Mar 2012 11:37:17 +0200 > >>> > >>> 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/kernel/syscalls/get_robust_list/get_robust_list01.c#L155 > >> 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. Didn't mean to offend :) > I just wonder which is the right thing. > Should we check euid or uid ? You mean that checking uid instead of > checking euid for a long time, right? Yup, and I agree it seems wrong. -serge ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [tip:core/locking] futex: Do not leak robust list to unprivileged process 2012-06-19 3:13 ` Serge Hallyn @ 2012-06-19 3:21 ` Wanlong Gao 2012-06-19 12:23 ` Serge Hallyn 0 siblings, 1 reply; 78+ messages in thread From: Wanlong Gao @ 2012-06-19 3:21 UTC (permalink / raw) To: Serge Hallyn Cc: mingo, hpa, linux-kernel, dvhart, a.p.zijlstra, jkosina, ebiederm, dhowells, keescook, tglx, linux-tip-commits On 06/19/2012 11:13 AM, Serge Hallyn wrote: > Quoting Wanlong Gao (gaowanlong@cn.fujitsu.com): >> On 06/19/2012 10:24 AM, Serge Hallyn wrote: >>> Quoting Wanlong Gao (gaowanlong@cn.fujitsu.com): >>>> On 03/29/2012 05:55 PM, tip-bot for Kees Cook wrote: >>>>> Commit-ID: bdbb776f882f5ad431aa1e694c69c1c3d6a4a5b8 >>>>> Gitweb: http://git.kernel.org/tip/bdbb776f882f5ad431aa1e694c69c1c3d6a4a5b8 >>>>> Author: Kees Cook <keescook@chromium.org> >>>>> AuthorDate: Mon, 19 Mar 2012 16:12:53 -0700 >>>>> Committer: Thomas Gleixner <tglx@linutronix.de> >>>>> CommitDate: Thu, 29 Mar 2012 11:37:17 +0200 >>>>> >>>>> 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/kernel/syscalls/get_robust_list/get_robust_list01.c#L155 >>>> 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. > > Didn't mean to offend :) Sorry for my poor words, I didn't mean that, either. ;) > >> I just wonder which is the right thing. >> Should we check euid or uid ? You mean that checking uid instead of >> checking euid for a long time, right? > > Yup, and I agree it seems wrong. Are there any other places where also switch checking uid instead of euid ? In this place, anyway, this syscall is already marked as deprecated. Thanks, Wanlong Gao > > -serge > ^ permalink raw reply [flat|nested] 78+ messages in thread
* Re: [tip:core/locking] futex: Do not leak robust list to unprivileged process 2012-06-19 3:21 ` Wanlong Gao @ 2012-06-19 12:23 ` Serge Hallyn 0 siblings, 0 replies; 78+ messages in thread From: Serge Hallyn @ 2012-06-19 12:23 UTC (permalink / raw) To: Wanlong Gao Cc: mingo, hpa, linux-kernel, dvhart, a.p.zijlstra, jkosina, ebiederm, dhowells, keescook, tglx, linux-tip-commits Quoting Wanlong Gao (gaowanlong@cn.fujitsu.com): > On 06/19/2012 11:13 AM, Serge Hallyn wrote: > > Quoting Wanlong Gao (gaowanlong@cn.fujitsu.com): > >> On 06/19/2012 10:24 AM, Serge Hallyn wrote: > >>> Quoting Wanlong Gao (gaowanlong@cn.fujitsu.com): > >>>> On 03/29/2012 05:55 PM, tip-bot for Kees Cook wrote: > >>>>> Commit-ID: bdbb776f882f5ad431aa1e694c69c1c3d6a4a5b8 > >>>>> Gitweb: http://git.kernel.org/tip/bdbb776f882f5ad431aa1e694c69c1c3d6a4a5b8 > >>>>> Author: Kees Cook <keescook@chromium.org> > >>>>> AuthorDate: Mon, 19 Mar 2012 16:12:53 -0700 > >>>>> Committer: Thomas Gleixner <tglx@linutronix.de> > >>>>> CommitDate: Thu, 29 Mar 2012 11:37:17 +0200 > >>>>> > >>>>> 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/kernel/syscalls/get_robust_list/get_robust_list01.c#L155 > >>>> 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. > > > > Didn't mean to offend :) > > Sorry for my poor words, I didn't mean that, either. ;) > > > > >> I just wonder which is the right thing. > >> Should we check euid or uid ? You mean that checking uid instead of > >> checking euid for a long time, right? > > > > Yup, and I agree it seems wrong. > > Are there any other places where also switch checking uid instead of euid ? > In this place, anyway, this syscall is already marked as deprecated. This isn't just this syscall, though, it's ptrace_may_access() which is used in quite a few places (20 at quick glance). -serge ^ permalink raw reply [flat|nested] 78+ messages in thread
end of thread, other threads:[~2012-08-03 18:06 UTC | newest] Thread overview: 78+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-03-19 23:12 [PATCH] futex: do not leak robust list to unprivileged process Kees Cook 2012-03-19 23:12 ` [kernel-hardening] " Kees Cook 2012-03-20 13:31 ` Serge Hallyn 2012-03-20 13:31 ` [kernel-hardening] " Serge Hallyn 2012-03-20 17:02 ` Thomas Gleixner 2012-03-20 17:02 ` [kernel-hardening] " Thomas Gleixner 2012-03-20 17:11 ` Kees Cook 2012-03-20 17:11 ` [kernel-hardening] " Kees Cook 2012-03-20 17:23 ` Ingo Molnar 2012-03-20 17:23 ` [kernel-hardening] " Ingo Molnar 2012-03-22 23:46 ` Thomas Gleixner 2012-03-22 23:46 ` [kernel-hardening] " Thomas Gleixner 2012-03-23 17:58 ` [PATCH] futex: mark get_robust_list as deprecated Kees Cook 2012-03-23 17:58 ` [kernel-hardening] " Kees Cook 2012-03-23 18:27 ` Thomas Gleixner 2012-03-23 18:27 ` [kernel-hardening] " Thomas Gleixner 2012-03-23 19:08 ` Kees Cook 2012-03-23 19:08 ` [kernel-hardening] " Kees Cook 2012-03-23 19:08 ` [PATCH v2] " Kees Cook 2012-03-23 19:08 ` [kernel-hardening] " Kees Cook [not found] ` <20120323190855.GA27213-0X9Bc/hWBUTk6RaD4rd5nQ@public.gmane.org> 2012-03-23 22:06 ` Eric W. Biederman 2012-03-23 22:06 ` [kernel-hardening] " Eric W. Biederman 2012-03-23 22:06 ` Eric W. Biederman [not found] ` <m1fwczvuph.fsf-+imSwln9KH6u2/kzUuoCbdi2O/JbrIOy@public.gmane.org> 2012-03-23 22:10 ` Kees Cook 2012-03-23 22:10 ` [kernel-hardening] " Kees Cook 2012-03-23 22:10 ` Kees Cook 2012-03-30 5:05 ` Matt Helsley 2012-03-30 5:05 ` [kernel-hardening] " Matt Helsley 2012-03-30 5:05 ` Matt Helsley [not found] ` <20120330050544.GA32299-52DBMbEzqgQ/wnmkkaCWp/UQ3DHhIser@public.gmane.org> 2012-03-30 6:14 ` Pavel Emelyanov 2012-03-30 6:14 ` [kernel-hardening] " Pavel Emelyanov 2012-03-30 6:14 ` Pavel Emelyanov 2012-03-30 22:51 ` Gene Cooperman 2012-03-30 22:51 ` Gene Cooperman 2012-03-30 22:51 ` [kernel-hardening] " Gene Cooperman 2012-03-27 18:05 ` Josh Boyer 2012-03-27 18:05 ` [kernel-hardening] " Josh Boyer 2012-03-27 19:13 ` Peter Zijlstra 2012-03-27 19:13 ` [kernel-hardening] " Peter Zijlstra 2012-03-29 9:56 ` [tip:core/locking] futex: Mark " tip-bot for Kees Cook 2012-08-02 10:35 ` [PATCH v2] futex: mark " richard -rw- weinberger 2012-08-02 10:35 ` [kernel-hardening] " richard -rw- weinberger 2012-08-02 11:11 ` Eric W. Biederman 2012-08-02 11:11 ` [kernel-hardening] " Eric W. Biederman 2012-08-03 10:17 ` richard -rw- weinberger 2012-08-03 10:17 ` [kernel-hardening] " richard -rw- weinberger 2012-08-03 11:02 ` Cyrill Gorcunov 2012-08-03 11:02 ` [kernel-hardening] " Cyrill Gorcunov 2012-08-03 11:19 ` richard -rw- weinberger 2012-08-03 11:19 ` [kernel-hardening] " richard -rw- weinberger 2012-08-03 11:27 ` Cyrill Gorcunov 2012-08-03 11:27 ` [kernel-hardening] " Cyrill Gorcunov 2012-08-03 11:30 ` richard -rw- weinberger 2012-08-03 11:30 ` [kernel-hardening] " richard -rw- weinberger 2012-08-03 11:35 ` Cyrill Gorcunov 2012-08-03 11:35 ` [kernel-hardening] " Cyrill Gorcunov 2012-08-03 11:38 ` richard -rw- weinberger 2012-08-03 11:38 ` [kernel-hardening] " richard -rw- weinberger 2012-08-03 12:38 ` Pavel Emelyanov 2012-08-03 12:38 ` [kernel-hardening] " Pavel Emelyanov 2012-08-03 12:58 ` Eric W. Biederman 2012-08-03 12:58 ` [kernel-hardening] " Eric W. Biederman 2012-08-03 13:00 ` richard -rw- weinberger 2012-08-03 13:00 ` [kernel-hardening] " richard -rw- weinberger 2012-08-03 17:16 ` Kees Cook 2012-08-03 17:16 ` [kernel-hardening] " Kees Cook 2012-08-03 18:06 ` [kernel-hardening] [PATCH] Revert "futex: Mark get_robust_list as deprecated" Richard Weinberger 2012-03-28 18:33 ` [PATCH] futex: do not leak robust list to unprivileged process Kees Cook 2012-03-28 18:33 ` [kernel-hardening] " Kees Cook 2012-03-28 21:24 ` Thomas Gleixner 2012-03-28 21:24 ` [kernel-hardening] " Thomas Gleixner 2012-03-29 9:55 ` [tip:core/locking] futex: Do " tip-bot for Kees Cook 2012-06-19 1:41 ` Wanlong Gao 2012-06-19 2:24 ` Serge Hallyn 2012-06-19 2:32 ` Wanlong Gao 2012-06-19 3:13 ` Serge Hallyn 2012-06-19 3:21 ` Wanlong Gao 2012-06-19 12:23 ` Serge Hallyn
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.