Re: [PATCH v9 02/15] livepatch: use `-z unique-symbol` if available to nuke pos-based search - Fāng-ruì Sòng

From: "Fāng-ruì Sòng" <maskray@google.com>
To: Alexander Lobakin <alexandr.lobakin@intel.com>
Cc: Miroslav Benes <mbenes@suse.cz>, Borislav Petkov <bp@alien8.de>,
	linux-hardening@vger.kernel.org, x86@kernel.org,
	Jesse Brandeburg <jesse.brandeburg@intel.com>,
	Kristen Carlson Accardi <kristen@linux.intel.com>,
	Kees Cook <keescook@chromium.org>,
	Miklos Szeredi <miklos@szeredi.hu>,
	Ard Biesheuvel <ardb@kernel.org>, Tony Luck <tony.luck@intel.com>,
	Bruce Schlobohm <bruce.schlobohm@intel.com>,
	Jessica Yu <jeyu@kernel.org>, kernel test robot <lkp@intel.com>,
	Evgenii Shatokhin <eshatokhin@virtuozzo.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Michal Marek <michal.lkml@markovi.net>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	"David S. Miller" <davem@davemloft.net>,
	Thomas Gleixner <tglx@linutronix.de>,
	Will Deacon <will@kernel.org>, Ingo Molnar <mingo@redhat.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Andy Lutomirski <luto@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Arnd Bergmann <arnd@arndb.de>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Nathan Chancellor <nathan@kernel.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Marios Pomonis <pomonis@google.com>,
	Sami Tolvanen <samitolvanen@google.com>,
	"H.J. Lu" <hjl.tools@gmail.com>, Nicolas Pitre <nico@fluxnic.net>,
	linux-kernel@vger.kernel.org, linux-kbuild@vger.kernel.org,
	linux-arch@vger.kernel.org, live-patching@vger.kernel.org,
	llvm@lists.linux.dev
Subject: Re: [PATCH v9 02/15] livepatch: use `-z unique-symbol` if available to nuke pos-based search
Date: Tue, 4 Jan 2022 19:24:56 -0800	[thread overview]
Message-ID: <20220105032456.hs3od326sdl4zjv4@google.com> (raw)
In-Reply-To: <20220103160615.7904-1-alexandr.lobakin@intel.com>

On 2022-01-03, Alexander Lobakin wrote:
>From: Miroslav Benes <mbenes@suse.cz>
>Date: Mon, 3 Jan 2022 14:55:42 +0100 (CET)
>
>> On Thu, 30 Dec 2021, Fāng-ruì Sòng wrote:
>>
>> > On Thu, Dec 30, 2021 at 3:11 AM Borislav Petkov <bp@alien8.de> wrote:
>> > >
>> > > On Thu, Dec 23, 2021 at 01:21:56AM +0100, Alexander Lobakin wrote:
>> > > > [PATCH v9 02/15] livepatch: use `-z unique-symbol` if available to nuke pos-based search
>>
>> ...
>>
>> > Apologies since I haven't read the patch series.
>> >
>> > The option does not exist in ld.lld and I am a bit concerning about
>> > its semantics: https://maskray.me/blog/2020-11-15-explain-gnu-linker-options#z-unique-symbol
>> >
>> > I thought that someone forwarded my comments (originally posted months
>> > on a feature request ago) here but seems not.
>> > (I am a ld.lld maintainer.)
>>
>> Do you mean
>> https://lore.kernel.org/all/20210123225928.z5hkmaw6qjs2gu5g@google.com/T/#u
>> ?
>>
>> Unfortunately, it did not lead anywhere. I think that '-z unique-symbol'
>> option should work fine as long as the live patching is concerned. Maybe I
>> misunderstood but your concerns mentioned at the blog do not apply. The
>> stability is not an issue for us since we (KLP) always work with already
>> built and fixed kernel. And(at least) GCC already uses number suffices for
>> IPA clones and it has not been a problem anywhere.

The stability problem may not happen frequently but is possible if the
compiler performs some IPA with new code.

Such disturbence is probably more likely with LTO or PGO.
For Clang LTO, Makefile currently specifies -mllvm -import-instr-limit=5.
If a function close to the boundary happens to cross the boundary,
if inlined into other translation units, the stability issue may affect
many translation units.

>LLD doesn't have such an option, so FG-KASLR + livepatching builds
>wouldn't be available for LLVM with the current approach (or we'd
>still need a stub that prints "FG-KASLR is not compatible with
>sympos != 0").
>Unfortunately, I discovered this a bit late, just after sending this
>revision.
>
>OTOH, there's no easy alternative. <file + function> pair looks
>appealing, but is it even possible for now to implement in the
>kernel without much refactoring?

<file + symbol> pair looks good to me and will solve the stability problem.