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: <20181026130442.GB10581@redhat.com>
Date:   Fri, 26 Oct 2018 15:04:42 +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/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.