Re: KASAN: use-after-free Read in task_is_descendant

Oleg Nesterov
2018-10-25T11:52:52Z

On 10/25, Kees Cook wrote:
>
> task_is_descendant() is called under rcu_read_lock() in both
> ptracer_exception_found() and yama_ptrace_access_check() so I don't
> understand how any of the tasks could get freed? This is walking
> group_leader and real_parent -- are these not stable under rcu_lock()?

group_leader/real_parent/etc are no longer rcu-protected after the exiting
child calls release_task() which in particular removes the child from
children/thread_group lists.

OK. Suppose you have an rcu-protected list, and each element also has a
reference counter so you can do something

struct elt {
atomic_t ctr;
struct list_head list;
int pid;
};

rcu_read_lock();
list_for_each_entry(elt, &LIST, list) {
if (elt->pid == 100) {
atomic_inc(&elt->ctr); // get_task_struct()
break;
}
}
rcu_read_unlock();

do_something(elt);

This code is fine. This elt can't be freed, you have a reference. But once
you drop rcu lock you can't trust elt->list.next! So, for example, you can
not do

rcu_read_lock();
list_for_each_entry_continue_rcu(elt, &LIST, list) {
...
}
rcu_read_unlock();

too late, elt.list.next can be already freed, or it can be freed while you
iterate the list.

Another simple example. Suppose you have a global PTR protected by rcu. So
ignoring the necessary rcu_dereference this code is fine:

rcu_read_lock();
if (ptr = PTR)
do_something(ptr);
rcu_read_unlcok();

But this is not:

ptr = PTR;
rcu_read_lock();
if (ptr)
do_something(ptr);
rcu_read_unlock();

basically the same thing...

Oleg.