linux-kernel - Re: current_thread_info() not respecting program order with gcc 4.8.x

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, 21 Nov 2013 14:32:34 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Alexander Holler <holler@...oftware.de>
Cc:	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
	Jakub Jelinek <jakub@...hat.com>,
	Richard Henderson <rth@...ddle.net>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Will Deacon <will.deacon@....com>,
	Catalin Marinas <catalin.marinas@....com>,
	Peter Zijlstra <peterz@...radead.org>,
	lttng-dev@...ts.lttng.org, Nathan Lynch <Nathan_Lynch@...tor.com>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Luis Lozano <llozano@...omium.org>,
	Bhaskar Janakiraman <bjanakiraman@...omium.org>,
	Han Shen <shenhan@...omium.org>
Subject: Re: current_thread_info() not respecting program order with gcc 4.8.x

On Thu, Nov 21, 2013 at 8:02 AM, Alexander Holler <holler@...oftware.de> wrote:
>
> Luis Lozano just noted (see https://lkml.org/lkml/2013/11/20/625) that
> current_thread_info() has the prototype
>
> static inline struct thread_info *current_thread_info(void)
> __attribute_const__;
>
> on arm (and arm64 and unicore32, something the paste from Mathieu missed so
> most people here might have missed that detail too). It's a very good
> finding from Luis.

No, because it is immaterial.

We *want* gcc to optimize away multiple accesses to "sp". Because it
doesn't *matter* whether "sp" changes or not, the *result* is always
the same. That's what the "const" means.

The "& ~(THREAD_SIZE - 1)" part will remove all the bits that can
change. Really. So the result *is* constant (within one thread).
Marking it constant and telling gcc that it can combine these things
is correct.

Guys, read my email again.

The bug is not that gcc can re-order or combine the accesses to "sp".
WE WANT THAT TO HAPPEN.

The bug is *outside* that "current_thread_info()" macro/inline
function. It's the *dereference* of the pointer that gcc re-orders.
AND THAT IS WRONG.

Gcc seems to mess up the alias analysis, and decide that the
deferences cannot alias. Which is wrong. They clearly *can* alias,
exactly because the value of "sp & ~(THREAD_SIZE - 1)" ends up having
the same value all the time.

                 Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/