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

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: <20160119093535.GA2458@gmail.com>
Date:	Tue, 19 Jan 2016 10:35:35 +0100
From:	Ingo Molnar <mingo@...nel.org>
To:	Gavin Guo <gavin.guo@...onical.com>
Cc:	Peter Zijlstra <peterz@...radead.org>,
	linux-kernel <linux-kernel@...r.kernel.org>, linux-mm@...ck.org,
	Jay Vosburgh <jay.vosburgh@...onical.com>,
	Liang Chen <liang.chen@...onical.com>, mgorman@...e.de,
	mingo@...hat.com, riel@...hat.com
Subject: Re: [PATCH V2] sched/numa: Fix use-after-free bug in the
 task_numa_compare


* Gavin Guo <gavin.guo@...onical.com> wrote:

> Hi Peter,
> 
> On Tue, Jan 19, 2016 at 1:13 AM, Peter Zijlstra <peterz@...radead.org> wrote:
> > On Mon, Jan 18, 2016 at 11:24:21PM +0800, gavin.guo@...onical.com wrote:
> >> From: Gavin Guo <gavin.guo@...onical.com>
> >>
> >> 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@...onical.com>
> >> Signed-off-by: Liang Chen <liangchen.linux@...il.com>
> >
> > 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@...il.com>

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