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]

Date:	Tue, 19 Jan 2016 07:40:18 +0800
From:	Gavin Guo <gavin.guo@...onical.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	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

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>