linux-kernel - Re: [PATCH] sched/fair: do not announce throttled next buddy in dequeue_task

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:	Mon, 11 Jul 2016 15:26:34 +0300
From:	Konstantin Khlebnikov <khlebnikov@...dex-team.ru>
To:	xlpang@...hat.com, Wanpeng Li <kernellwp@...il.com>
Cc:	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	stable@...r.kernel.org
Subject: Re: [PATCH] sched/fair: do not announce throttled next buddy in
 dequeue_task_fair

On 11.07.2016 15:12, Xunlei Pang wrote:
> On 2016/07/11 at 17:54, Wanpeng Li wrote:
>> Hi Konstantin, Xunlei,
>> 2016-07-11 16:42 GMT+08:00 Xunlei Pang <xpang@...hat.com>:
>>> On 2016/07/11 at 16:22, Xunlei Pang wrote:
>>>> On 2016/07/11 at 15:25, Wanpeng Li wrote:
>>>>> 2016-06-16 20:57 GMT+08:00 Konstantin Khlebnikov <khlebnikov@...dex-team.ru>:
>>>>>> Hierarchy could be already throttled at this point. Throttled next
>>>>>> buddy could trigger null pointer dereference in pick_next_task_fair().
>>>>> There is cfs_rq->next check in pick_next_entity(), so how can null
>>>>> pointer dereference happen?
>>>> I guess it's the following code leading to a NULL se returned:
>>> s/NULL/empty-entity cfs_rq se/
>>>
>>>> pick_next_entity():
>>>>      if (cfs_rq->next && wakeup_preempt_entity(cfs_rq->next, left) < 1)
>>              ^^^^^^^^^^^^^
>> I think this will return false.
>
> With the wrong throttled_hierarchy(), I think this can happen. But after we have the
> corrected throttled_hierarchy() patch, I can't see how it is possible.
>
> dequeue_task_fair():
>      if (task_sleep && parent_entity(se))
>          set_next_buddy(parent_entity(se));
>
> How does dequeue_task_fair() with DEQUEUE_SLEEP set(true task_sleep) happen to a throttled hierarchy?
> IOW, a task belongs to a throttled hierarchy is running?
>
> Maybe Konstantin knows the reason.

This function (dequeue_task_fair) check throttling but at point it could skip several
levels and announce as next buddy actually throttled entry.
Probably this bug hadn't happened but this's really hard to prove that this is impossible.
->set_curr_task(), PI-boost or some tricky migration in balancer could break this easily.

-- 
Konstantin