linux-kernel - Re: Linux 3.19-rc5

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:	Thu, 29 Jan 2015 17:25:07 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Ingo Molnar <mingo@...nel.org>
Cc:	Peter Hurley <peter@...leysoftware.com>,
	Davidlohr Bueso <dave@...olabs.net>,
	Peter Zijlstra <peterz@...radead.org>,
	Bruno Prémont <bonbons@...ux-vserver.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ilya Dryomov <ilya.dryomov@...tank.com>,
	Mike Galbraith <umgwanakikbuti@...il.com>,
	Oleg Nesterov <oleg@...hat.com>
Subject: Re: Linux 3.19-rc5

On Thu, Jan 29, 2015 at 5:12 PM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> The WARN() was already changed to a WARN_ONCE().

Oh, but I notice that the "__set_current_state(TASK_RUNNING) ends up
always happening.

So I think the right fix is to:

 - warn once like we do

 - but *not* do that __set_current_state() which was always total crap anyway

Why do I say "total crap"? Because of two independent issues:

 (a) it actually changes behavior for a debug vs non-debug kernel,
which is a really bad idea to begin with

 (b) it's really wrong. The whole "nested sleep" case was never a
major bug to begin with, just a possible inefficiency where constant
nested sleeps would possibly make the outer sleep not sleep. But that
"could possibly make" case was the unlikely case, and the debug patch
made it happen *all* the time by explicitly setting things running.

So I think the proper patch is the attached.

The comment is also crap. The comment says

    "Blocking primitives will set (and therefore destroy) current->state [...]"

but the reality is that they *may* set it, and only in the unlikely
slow-path where they actually block.

So doing this in "__may_sleep()" is just bogus and horrible horrible
crap. It turns the "harmless ugliness" into a real *harmful* bug. The
key word of "__may_sleep()" is that "MAY" part. It's a debug thing to
make relatively rare cases show up.

PeterZ, please don't make "debugging" patches like this. Ever again.
Because this was just stupid, and it took me too long to realize that
despite the warning being shut up, the debug patch was still actively
doing bad bad things.

Ingo, maybe you'd want to apply this through the scheduler tree, the
way you already did the WARN_ONCE() thing.

Bruno, does this finally actually fix your pccard thing?

                              Linus

View attachment "patch.diff" of type "text/plain" (845 bytes)