Re: [PATCH v9 02/15] livepatch: use `-z unique-symbol` if available to nuke pos-based search

From: Fāng-ruì Sòng
Date: Tue Jan 04 2022 - 22:25:13 EST


On 2022-01-03, Alexander Lobakin wrote:
From: Miroslav Benes <mbenes@xxxxxxx>
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@xxxxxxxxx> 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@xxxxxxxxxx/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.