Kernel-hardening archive on lore.kernel.org
 help / color / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Marco Elver <elver@google.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>,
	Sami Tolvanen <samitolvanen@google.com>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Will Deacon <will@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	Kees Cook <keescook@chromium.org>,
	clang-built-linux <clang-built-linux@googlegroups.com>,
	Kernel Hardening <kernel-hardening@lists.openwall.com>,
	linux-arch <linux-arch@vger.kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-pci@vger.kernel.org,
	"maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
	<x86@kernel.org>
Subject: Re: [PATCH 00/22] add support for Clang LTO
Date: Tue, 30 Jun 2020 22:12:43 +0200
Message-ID: <20200630201243.GD4817@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20200630191931.GA884155@elver.google.com>

On Tue, Jun 30, 2020 at 09:19:31PM +0200, Marco Elver wrote:
> I was asked for input on this, and after a few days digging through some
> history, thought I'd comment. Hope you don't mind.

Not at all, being the one that asked :-)

> First of all, I agree with the concerns, but not because of LTO.
> 
> To set the stage better, and summarize the fundamental problem again:
> we're in the unfortunate situation that no compiler today has a way to
> _efficiently_ deal with C11's memory_order_consume
> [https://lwn.net/Articles/588300/]. If we did, we could just use that
> and be done with it. But, sadly, that doesn't seem possible right now --
> compilers just say consume==acquire.

I'm not convinced C11 memory_order_consume would actually work for us,
even if it would work. That is, given:

  https://lore.kernel.org/lkml/20150520005510.GA23559@linux.vnet.ibm.com/

only pointers can have consume, but like I pointed out, we have code
that relies on dependent loads from integers.

> Will suggests doing the same in the
> kernel: https://lkml.kernel.org/r/20200630173734.14057-19-will@kernel.org

PowerPC would need a similar thing, it too will not preserve causality
for control dependecies.

> What we're most worried about right now is the existence of compiler
> transformations that could break data dependencies by e.g. turning them
> into control dependencies.

Correct.

> If this is a real worry, I don't think LTO is the magical feature that
> will uncover those optimizations. If these compiler transformations are
> real, they also exist in a normal build! 

Agreed, _however_ with the caveat that LTO could make them more common.

After all, with whole program analysis, the compiler might be able to
more easily determine that our pointer @ptr is only ever assigned the
values of &A, &B or &C, while without that visibility it would not be
able to determine this.

Once it knows @ptr has a limited number of determined values, the
conversion into control dependencies becomes much more likely.

> And if we are worried about them, we need to stop relying on dependent
> load ordering across the board; or switch to -O0 for everything.
> Clearly, we don't want either.

Agreed.

> Why do we think LTO is special?

As argued above, whole-program analysis would make it more likely. But I
agree the fundamental problem exists independent from LTO.

> But as far as we can tell, there is no evidence of the dreaded "data
> dependency to control dependency" conversion with LTO that isn't there
> in non-LTO builds, if it's even there at all. Has the data to control
> dependency conversion been encountered in the wild? If not, is the
> resulting reaction an overreaction? If so, we need to be careful blaming
> LTO for something that it isn't even guilty of.

It is mostly paranoia; in a large part driven by the fact that even if
such a conversion were to be done, it could go a very long time without
actually causing problems, and longer still for such problems to be
traced back to such an 'optimization'.

That is, the collective hurt from debugging too many ordering issues.

> So, we are probably better off untangling LTO from the story:
> 
> 1. LTO or no LTO does not matter. The LTO series should not get tangled
>    up with memory model issues.
> 
> 2. The memory model question and problems need to be answered and
>    addressed separately.
> 
> Thoughts?

How hard would it be to creates something that analyzes a build and
looks for all 'dependent load -> control dependency' transformations
headed by a volatile (and/or from asm) load and issues a warning for
them?

This would give us an indication of how valuable this transformation is
for the kernel. I'm hoping/expecting it's vanishingly rare, but what do
I know.


  reply index

Thread overview: 99+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-24 20:31 Sami Tolvanen
2020-06-24 20:31 ` [PATCH 01/22] objtool: use sh_info to find the base for .rela sections Sami Tolvanen
2020-06-24 20:31 ` [PATCH 02/22] kbuild: add support for Clang LTO Sami Tolvanen
2020-06-24 20:53   ` Nick Desaulniers
2020-06-24 21:29     ` Sami Tolvanen
2020-06-25  2:26   ` Nathan Chancellor
2020-06-25 16:13     ` Sami Tolvanen
2020-06-24 20:31 ` [PATCH 03/22] kbuild: lto: fix module versioning Sami Tolvanen
2020-06-24 20:31 ` [PATCH 04/22] kbuild: lto: fix recordmcount Sami Tolvanen
2020-06-24 21:27   ` Peter Zijlstra
2020-06-24 21:45     ` Sami Tolvanen
2020-06-25  7:45       ` Peter Zijlstra
2020-06-25 16:15         ` Sami Tolvanen
2020-06-25 20:02           ` [RFC][PATCH] objtool,x86_64: Replace recordmcount with objtool Peter Zijlstra
2020-06-25 20:54             ` Nick Desaulniers
2020-06-25 22:40             ` Sami Tolvanen
2020-06-26 11:29               ` Peter Zijlstra
2020-06-26 11:42                 ` Peter Zijlstra
2020-06-24 20:31 ` [PATCH 05/22] kbuild: lto: postpone objtool Sami Tolvanen
2020-06-24 21:19   ` Peter Zijlstra
2020-06-24 21:49     ` Sami Tolvanen
2020-06-25  7:47       ` Peter Zijlstra
2020-06-25 16:22         ` Sami Tolvanen
2020-06-25 18:33           ` Peter Zijlstra
2020-06-25 19:32             ` Sami Tolvanen
2020-06-24 20:31 ` [PATCH 06/22] kbuild: lto: limit inlining Sami Tolvanen
2020-06-24 21:20   ` Peter Zijlstra
2020-06-24 23:37     ` Sami Tolvanen
2020-06-24 20:31 ` [PATCH 07/22] kbuild: lto: merge module sections Sami Tolvanen
2020-06-24 21:01   ` Nick Desaulniers
2020-06-24 21:31     ` Sami Tolvanen
2020-06-24 20:31 ` [PATCH 08/22] kbuild: lto: remove duplicate dependencies from .mod files Sami Tolvanen
2020-06-24 21:13   ` Nick Desaulniers
2020-06-24 20:31 ` [PATCH 09/22] init: lto: ensure initcall ordering Sami Tolvanen
2020-06-25  0:58   ` kernel test robot
2020-06-25  4:19   ` kernel test robot
2020-06-24 20:31 ` [PATCH 10/22] init: lto: fix PREL32 relocations Sami Tolvanen
2020-06-24 20:31 ` [PATCH 11/22] pci: " Sami Tolvanen
2020-06-24 22:49   ` kernel test robot
2020-06-24 23:03     ` Nick Desaulniers
2020-06-24 23:21       ` Sami Tolvanen
2020-06-24 20:31 ` [PATCH 12/22] modpost: lto: strip .lto from module names Sami Tolvanen
2020-06-24 22:05   ` Nick Desaulniers
2020-06-24 20:31 ` [PATCH 13/22] scripts/mod: disable LTO for empty.c Sami Tolvanen
2020-06-24 20:57   ` Nick Desaulniers
2020-06-24 20:31 ` [PATCH 14/22] efi/libstub: disable LTO Sami Tolvanen
2020-06-24 20:31 ` [PATCH 15/22] drivers/misc/lkdtm: disable LTO for rodata.o Sami Tolvanen
2020-06-24 20:31 ` [PATCH 16/22] arm64: export CC_USING_PATCHABLE_FUNCTION_ENTRY Sami Tolvanen
2020-06-24 20:31 ` [PATCH 17/22] arm64: vdso: disable LTO Sami Tolvanen
2020-06-24 20:58   ` Nick Desaulniers
2020-06-24 21:09     ` Nick Desaulniers
2020-06-24 23:51       ` Andi Kleen
2020-06-24 21:52     ` Sami Tolvanen
2020-06-24 23:05       ` Nick Desaulniers
2020-06-24 23:39         ` Sami Tolvanen
2020-06-24 20:31 ` [PATCH 18/22] arm64: allow LTO_CLANG and THINLTO to be selected Sami Tolvanen
2020-06-24 20:31 ` [PATCH 19/22] x86, vdso: disable LTO only for vDSO Sami Tolvanen
2020-06-24 20:31 ` [PATCH 20/22] x86, ftrace: disable recordmcount for ftrace_make_nop Sami Tolvanen
2020-06-24 20:31 ` [PATCH 21/22] x86, relocs: Ignore L4_PAGE_OFFSET relocations Sami Tolvanen
2020-06-24 20:32 ` [PATCH 22/22] x86, build: allow LTO_CLANG and THINLTO to be selected Sami Tolvanen
2020-06-24 21:15 ` [PATCH 00/22] add support for Clang LTO Peter Zijlstra
2020-06-24 21:30   ` Sami Tolvanen
2020-06-25  8:27     ` Will Deacon
2020-06-24 21:31   ` Nick Desaulniers
2020-06-25  8:03     ` Peter Zijlstra
2020-06-25  8:24       ` Peter Zijlstra
2020-06-25  8:57         ` Peter Zijlstra
2020-06-30 19:19           ` Marco Elver
2020-06-30 20:12             ` Peter Zijlstra [this message]
2020-06-30 20:30               ` Paul E. McKenney
2020-07-01  9:10                 ` Peter Zijlstra
2020-07-01 14:20                   ` David Laight
2020-07-01 16:06                     ` Paul E. McKenney
2020-07-02  9:37                       ` David Laight
2020-07-02 18:00                         ` Paul E. McKenney
2020-07-01  9:41                 ` Marco Elver
2020-07-01 10:03                   ` Will Deacon
2020-07-01 11:40                   ` Peter Zijlstra
2020-07-01 14:06                     ` Paul E. McKenney
2020-07-01 15:05                       ` Peter Zijlstra
2020-07-01 16:03                         ` Paul E. McKenney
2020-07-02  8:20                           ` Peter Zijlstra
2020-07-02 17:59                             ` Paul E. McKenney
2020-07-03 13:13                               ` Peter Zijlstra
2020-07-03 13:25                                 ` Peter Zijlstra
2020-07-03 14:51                                   ` Paul E. McKenney
2020-07-03 14:42                                 ` Paul E. McKenney
2020-07-06 16:26                                   ` Paul E. McKenney
2020-07-06 18:29                                     ` Peter Zijlstra
2020-07-06 18:39                                       ` Paul E. McKenney
2020-07-06 19:40                                         ` Peter Zijlstra
2020-07-06 23:41                                           ` Paul E. McKenney
2020-06-28 16:56 ` Masahiro Yamada
2020-06-29 23:20   ` Sami Tolvanen
2020-07-07 15:51     ` Sami Tolvanen
2020-07-07 16:05       ` Sami Tolvanen
2020-07-07 16:56         ` Jakub Kicinski
2020-07-07 17:17           ` Nick Desaulniers
2020-07-07 17:30             ` Jakub Kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200630201243.GD4817@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=clang-built-linux@googlegroups.com \
    --cc=elver@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=masahiroy@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=paulmck@kernel.org \
    --cc=samitolvanen@google.com \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Kernel-hardening archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kernel-hardening/0 kernel-hardening/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kernel-hardening kernel-hardening/ https://lore.kernel.org/kernel-hardening \
		kernel-hardening@lists.openwall.com
	public-inbox-index kernel-hardening

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/com.openwall.lists.kernel-hardening


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git