Re: [PATCH V2] sched/numa: Fix use-after-free bug in the task_numa_compare

From: Ingo Molnar
Date: Tue Jan 19 2016 - 04:35:46 EST



* Gavin Guo <gavin.guo@xxxxxxxxxxxxx> wrote:

> Hi Peter,
>
> On Tue, Jan 19, 2016 at 1:13 AM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> > On Mon, Jan 18, 2016 at 11:24:21PM +0800, gavin.guo@xxxxxxxxxxxxx wrote:
> >> From: Gavin Guo <gavin.guo@xxxxxxxxxxxxx>
> >>
> >> The following message can be observed on the Ubuntu v3.13.0-65 with KASan
> >> backported:
> >
> > <snip>
> >
> >> As commit 1effd9f19324 ("sched/numa: Fix unsafe get_task_struct() in
> >> task_numa_assign()") points out, the rcu_read_lock() cannot protect the
> >> task_struct from being freed in the finish_task_switch(). And the bug
> >> happens in the process of calculation of imp which requires the access of
> >> p->numa_faults being freed in the following path:
> >>
> >> do_exit()
> >> current->flags |= PF_EXITING;
> >> release_task()
> >> ~~delayed_put_task_struct()~~
> >> schedule()
> >> ...
> >> ...
> >> rq->curr = next;
> >> context_switch()
> >> finish_task_switch()
> >> put_task_struct()
> >> __put_task_struct()
> >> task_numa_free()
> >>
> >> The fix here to get_task_struct() early before end of dst_rq->lock to
> >> protect the calculation process and also put_task_struct() in the
> >> corresponding point if finally the dst_rq->curr somehow cannot be
> >> assigned.
> >>
> >> v1->v2:
> >> - Fix coding style suggested by Peter Zijlstra.
> >>
> >> Signed-off-by: Gavin Guo <gavin.guo@xxxxxxxxxxxxx>
> >> Signed-off-by: Liang Chen <liangchen.linux@xxxxxxxxx>
> >
> > Argh, sorry for not noticing before; this SoB chain is not valid.
> >
> > Gavin wrote (per From) and send me the patch (per actual email headers),
> > so Liang never touched it.
> >
> > Should that be a reviewed-by for him?
>
> Liang is also the co-author of the original patch, we figured out the code
> by parallel programming, part of the idea was came from him. If SoB is
> not valid, can I change the line to the following?
>
> Co-authored-by: Liang Chen <liangchen.linux@xxxxxxxxx>

So unless you guys shared the same keyboard at the same time, there's at least
line granular authorship, right?

The main author (the guy who wrote the most code and comments) should be the
'From' author - additional help can be credited in the changelog. If of one you
wrote an initial version that the other one used, you can use something like:

Originally-From: ...

Thanks,

Ingo