linux-kernel - Re: [patch 17/18] sched: Enable might

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, 16 May 2017 09:33:52 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Peter Zijlstra <peterz@...radead.org>
cc:     Steven Rostedt <rostedt@...dmis.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Mark Rutland <mark.rutland@....com>
Subject: Re: [patch 17/18] sched: Enable might_sleep() checks early

On Tue, 16 May 2017, Peter Zijlstra wrote:
> On Mon, May 15, 2017 at 09:12:03PM +0200, Thomas Gleixner wrote:
> > On Mon, 15 May 2017, Steven Rostedt wrote:
> > 
> > > On Sun, 14 May 2017 20:27:33 +0200
> > > Thomas Gleixner <tglx@...utronix.de> wrote:
> > > 
> > > > might_sleep() checks are enabled after the boot process is done. That hides
> > > > bugs in the smp bringup and driver initialization code.
> > > > 
> > > > Enable it right when the scheduler starts working, i.e. when init task and
> > > > kthreadd have been created and right before the idle task enables
> > > > preemption.
> > > 
> > > Looking at commit b433c3d4549ae749, it appears that on very slow
> > > machines, there is a possibility that the init task can start running.
> > > Should system_state be updated before that complete() is called?
> > 
> > That commit is magic voodoo with exactly no effect at all.
> > 
> > rest_init() is called with preemption disabled and nothing can schedule
> > there _before_ schedule_preempt_disabled().
> > 
> > Both threads - init task and kthreadd - are only created and woken up. They
> > cannot get on the CPU simply because preemption is disabled. And this was
> > the case back then in 2.6.35 as well.
> > 
> > It does not matter at all whether the machine is slow or not. That
> > completion is pointless.
> > 
> > Peter, can you explain what the heck this patch is actually doing?
> 
> Argh.. what a shit Changelog, who wrote that crap!?

Indeed.

> So the problem was with PREEMPT_VOLUNTARY (where, as you know,
> preempt_disable() has no meaning).
> 
> Supposedly there's a might_sleep()/cond_resched() point somewhere around
> there (every alloc in the fork path for example), which will happily
> reschedule us.

Darn, forgot about PREEMPT_VOLUNTARY and that excellent changelog does not
mention it either. 

> So if we schedule to the kernel_init() task before we set kthreadd_task
> we'll try and spawn kthreads and OOPS.

So back to Stevens question. No, we can't set the state earlier than right
before schedule() simply because with PREEMPT preemption _is_ actually
disabled and kernel_kthread() will trigger might_sleep() splats.

What a mess.

Thanks,

	tglx