| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 23 May 2017 07:47:32 +0200
From: Takashi Iwai <tiwai@...e.de>
To: ebiederm@...ssion.com (Eric W. Biederman)
Cc: linux-kernel@...r.kernel.org
Subject: Re: [CFT][PATCH] ptrace: Properly initialize ptracer_cred on fork
On Mon, 22 May 2017 23:04:48 +0200,
Eric W. Biederman wrote:
>
>
> When I introduced ptracer_cred I failed to consider the weirdness of
> fork where the task_struct copies the old value by default. This
> winds up leaving ptracer_cred set even when a process forks and
> the child process does not wind up being ptraced.
>
> Because ptracer_cred is not set on non-ptraced processes whose
> parents were ptraced this has broken the ability of the enlightenment
> window manager to start setuid children.
>
> Fix this by properly initializing ptracer_cred in ptrace_init_task
>
> This must be done with a little bit of care to preserve the current value
> of ptracer_cred when ptrace carries through fork. Re-reading the
> ptracer_cred from the ptracing process at this point is inconsistent
> with how PT_PTRACE_CAP has been maintained all of these years.
>
> Fixes: 64b875f7ac8a ("ptrace: Capture the ptracer's creds not PT_PTRACE_CAP")
> Signed-off-by: "Eric W. Biederman" <ebiederm@...ssion.com>
> ---
>
> If I could get some folks to test and verify this fixes the
> enlightenment issue I would really appreciate it.
This seems giving a compile warning and it becomes error in the
following:
In file included from ./include/linux/mutex.h:13:0,
from ./include/linux/kernfs.h:13,
from ./include/linux/sysfs.h:15,
from ./include/linux/kobject.h:21,
from ./include/linux/device.h:17,
from drivers/gpu/drm/i915/gvt/kvmgt.c:32:
./include/linux/ptrace.h: In function ‘ptrace_init_task’:
./arch/x86/include/asm/current.h:17:17: error: passing argument 3 of ‘__ptrace_link’ discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
#define current get_current()
^
./include/linux/ptrace.h:210:41: note: in expansion of macro ‘current’
__ptrace_link(child, current->parent, current->ptracer_cred);
^~~~~~~
In file included from ./arch/x86/include/asm/stacktrace.h:10:0,
from ./arch/x86/include/asm/perf_event.h:246,
from ./include/linux/perf_event.h:24,
from ./arch/x86/include/asm/kvm_host.h:24,
from ./include/linux/kvm_host.h:37,
from drivers/gpu/drm/i915/gvt/kvmgt.c:41:
./include/linux/ptrace.h:56:13: note: expected ‘struct cred *’ but argument is of type ‘const struct cred *’
extern void __ptrace_link(struct task_struct *child,
^~~~~~~~~~~~~
cc1: all warnings being treated as errors
thanks,
Takashi
>
> include/linux/ptrace.h | 7 +++++--
> kernel/ptrace.c | 20 +++++++++++++-------
> 2 files changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
> index 422bc2e4cb6a..23c5716f7bd2 100644
> --- a/include/linux/ptrace.h
> +++ b/include/linux/ptrace.h
> @@ -54,7 +54,8 @@ extern int ptrace_request(struct task_struct *child, long request,
> unsigned long addr, unsigned long data);
> extern void ptrace_notify(int exit_code);
> extern void __ptrace_link(struct task_struct *child,
> - struct task_struct *new_parent);
> + struct task_struct *new_parent,
> + struct cred *ptracer_cred);
> extern void __ptrace_unlink(struct task_struct *child);
> extern void exit_ptrace(struct task_struct *tracer, struct list_head *dead);
> #define PTRACE_MODE_READ 0x01
> @@ -206,7 +207,7 @@ static inline void ptrace_init_task(struct task_struct *child, bool ptrace)
>
> if (unlikely(ptrace) && current->ptrace) {
> child->ptrace = current->ptrace;
> - __ptrace_link(child, current->parent);
> + __ptrace_link(child, current->parent, current->ptracer_cred);
>
> if (child->ptrace & PT_SEIZED)
> task_set_jobctl_pending(child, JOBCTL_TRAP_STOP);
> @@ -215,6 +216,8 @@ static inline void ptrace_init_task(struct task_struct *child, bool ptrace)
>
> set_tsk_thread_flag(child, TIF_SIGPENDING);
> }
> + else
> + child->ptracer_cred = NULL;
> }
>
> /**
> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> index 266ddcc1d8bb..79cbe00fe787 100644
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -60,19 +60,25 @@ int ptrace_access_vm(struct task_struct *tsk, unsigned long addr,
> }
>
>
> +void __ptrace_link(struct task_struct *child, struct task_struct *new_parent,
> + struct cred *ptracer_cred)
> +{
> + BUG_ON(!list_empty(&child->ptrace_entry));
> + list_add(&child->ptrace_entry, &new_parent->ptraced);
> + child->parent = new_parent;
> + child->ptracer_cred = get_cred(ptracer_cred);
> +}
> +
> /*
> * ptrace a task: make the debugger its new parent and
> * move it to the ptrace list.
> *
> * Must be called with the tasklist lock write-held.
> */
> -void __ptrace_link(struct task_struct *child, struct task_struct *new_parent)
> +static void ptrace_link(struct task_struct *child, struct task_struct *new_parent)
> {
> - BUG_ON(!list_empty(&child->ptrace_entry));
> - list_add(&child->ptrace_entry, &new_parent->ptraced);
> - child->parent = new_parent;
> rcu_read_lock();
> - child->ptracer_cred = get_cred(__task_cred(new_parent));
> + __ptrace_link(child, new_parent, __task_cred(new_parent));
> rcu_read_unlock();
> }
>
> @@ -386,7 +392,7 @@ static int ptrace_attach(struct task_struct *task, long request,
> flags |= PT_SEIZED;
> task->ptrace = flags;
>
> - __ptrace_link(task, current);
> + ptrace_link(task, current);
>
> /* SEIZE doesn't trap tracee on attach */
> if (!seize)
> @@ -459,7 +465,7 @@ static int ptrace_traceme(void)
> */
> if (!ret && !(current->real_parent->flags & PF_EXITING)) {
> current->ptrace = PT_PTRACED;
> - __ptrace_link(current, current->real_parent);
> + ptrace_link(current, current->real_parent);
> }
> }
> write_unlock_irq(&tasklist_lock);
> --
> 2.10.1
>
Powered by blists - more mailing lists