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

[Oleg Nesterov]

On 10/26, Tetsuo Handa wrote:
>
> Since the "child" passed to task_is_descendant() has at least one reference
> count taken by find_get_task_by_vpid(), rcu_dereference(walker->real_parent)
> in the first iteration
>
>   while (child->pid > 0) {
>     if (!thread_group_leader(child))
>       walker = rcu_dereference(child->group_leader);
>     if (walker == parent) {
>       rc = 1;
>       break;
>     }
>     walker = rcu_dereference(walker->real_parent);
>   }
>
> must not trigger use-after-free bug.

Yes,

> Thus, when this use-after-free was
> detected at rcu_dereference(walker->real_parent), the memory pointed by
> "walker" must have been released between
>
>   while (walker->pid > 0) {
>     if (!thread_group_leader(walker))
>       walker = rcu_dereference(walker->group_leader);
>
> and
>
>     walker = rcu_dereference(walker->real_parent);
>   }

this is possible too, rcu callback which frees the task_struct can run
int between

> because otherwise use-after-free would have been reported at walker->pid
> or thread_group_leader(walker) or rcu_dereference(walker->group_leader).

Well, I do not know how much kasan is "percise", but if it is "perfect"
then you are right,

> Then, what pid_alive(child) is testing?

I tried to explain this in my previous email. I even mentioned that we could
do another check, say, ->sighand != NULL, or list_empty(child->sibling) with
the same effect.

> Suppose p1 == p2->real_parent and p2 == p3->real_parent, and p1 exited
> when p2 tried to attach on p1, p2->real_parent was pointing to already
> (or about to be) freed p1.

No, p2->real_parent will be updated. If p1 exits it will re-parent its
children including p2.

Again, did you read my previous email?

Let me repeat, of course I can be wrong. But so far I am answering the
same questions again and again...

Oleg.