linux-kernel - Re: KASAN: use-after-free Read in task_is

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

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

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

Message-ID: <20181022134634.GA7358@redhat.com>
Date:   Mon, 22 Oct 2018 15:46:34 +0200
From:   Oleg Nesterov <oleg@...hat.com>
To:     Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Cc:     serge@...lyn.com,
        syzbot <syzbot+a9ac39bf55329e206219@...kaller.appspotmail.com>,
        jmorris@...ei.org, keescook@...omium.org,
        linux-kernel@...r.kernel.org,
        linux-security-module@...r.kernel.org,
        syzkaller-bugs@...glegroups.com
Subject: Re: KASAN: use-after-free Read in task_is_descendant

On 10/22, Tetsuo Handa wrote:
>
> > However, task_is_descendant() looks unnecessarily complicated, it could be
> >
> > 	static int task_is_descendant(struct task_struct *parent,
> > 				      struct task_struct *child)
> > 	{
> > 		int rc = 0;
> > 		struct task_struct *walker;
> >
> > 		if (!parent || !child)
> > 			return 0;
> >
> > 		rcu_read_lock();
> > 		for (walker = child; walker->pid; walker = rcu_dereference(walker->real_parent))
> > 			if (same_thread_group(parent, walker)) {
> > 				rc = 1;
> > 				break;
> > 			}
> > 		rcu_read_unlock();
> >
> > 		return rc;
> > 	}
> >
> > And again, I do not know how/if yama ensures that child is rcu-protected, perhaps
> > task_is_descendant() needs to check pid_alive(child) right after rcu_read_lock() ?
>
> Since the caller (ptrace() path) called get_task_struct(child), child itself can't be
> released. Do we still need pid_alive(child) ?

get_task_struct(child) can only ensure that this task_struct can't be freed.

Suppose that this child exits after get_task_struct(), then its real_parent exits
too and calls call_rcu(delayed_put_task_struct).

Now, when task_is_descendant() is called, rcu_read_lock() can happen after rcu gp,
iow child->parent can be already freed/reused/unmapped.

We need to ensure that child is still protected by RCU.

Oleg.