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

[Oleg Nesterov <oleg@redhat.com>]

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.