LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Kees Cook <email@example.com>
To: Alexander Lobakin <firstname.lastname@example.org>
Kristen C Accardi <email@example.com>,
Kristen Carlson Accardi <firstname.lastname@example.org>,
Masahiro Yamada <email@example.com>,
"H. Peter Anvin" <firstname.lastname@example.org>, Jessica Yu <email@example.com>,
Nathan Chancellor <firstname.lastname@example.org>,
Nick Desaulniers <email@example.com>,
Marios Pomonis <firstname.lastname@example.org>,
Sami Tolvanen <email@example.com>,
Tony Luck <firstname.lastname@example.org>, Ard Biesheuvel <email@example.com>,
Jesse Brandeburg <firstname.lastname@example.org>,
Lukasz Czapnik <email@example.com>,
Marta A Plantykow <firstname.lastname@example.org>,
Michal Kubiak <email@example.com>,
Michal Swiatkowski <firstname.lastname@example.org>,
Subject: Re: [PATCH v6 kspp-next 00/22] Function Granular KASLR
Date: Wed, 1 Sep 2021 18:36:59 -0700 [thread overview]
Message-ID: <202109011815.1C439A6DA9@keescook> (raw)
On Wed, Sep 01, 2021 at 12:36:58PM +0200, Alexander Lobakin wrote:
> Without FG-KASLR, we have only one .text section, and the total
> section number is relatively small.
> With FG-KASLR enabled, we have 40K+ separate text sections (I have
> 40K on a setup with ClangLTO and ClangCFI and about 48K on a
> "regular" one) and each of them is described in the ELF header. Plus
> a separate .rela.text section for every single of them. That's the
> main reason of the size increases.
If you have the size comparisons handy, I'd love to see them. My memory
from v5 was that none of that end up in-core. And in that case, why
limit the entropy of the resulting layout?
> We still have LD_ORPHAN_WARN on non-FG-KASLR builds, but we also
> have a rather different set of sections with FG-KASLR enabled. For
> example, I noticed the appearing of .symtab_shndx section only in
> virtue of LD_ORPHAN_WARN. So it's kinda not the same.
Agreed: I'd rather have LD_ORPHAN_WARN always enabled.
> I don't see a problem in this extra minute. FG-KASLR is all about
But not at this cost. Maybe the x86 maintainers will disagree, but I see
this as a prohibitive cost to doing development work under FGKASLR, and
if we expect this to become the default in distros, no one is going to
be happy with that change. Link time dominates the partial rebuild time,
so my opinion is that it should not be so inflated if not absolutely
needed. Perhaps once the link time bugs in ld.bfd and ld.lld get fixed,
but not now.
> security, and you often pay something for this. We already have a
> size increase, and a small delay while booting, and we can't get
> rid of them. With orphan sections you leave a space for potentional
There's a difference between development time costs and run time costs.
I don't think the LD_ORPHAN_WARN coverage is worth it in this case.
Either way, we need to fix the linker.
> flaws of the code, linker and/or linker script, which is really
> unwanted in case of a security feature.
> After all, ClangLTO increases the linking time at lot, and
> TRIM_UNUSED_KSYMS builds almost the entire kernel two times in a
> row, but nobody complains about this as there's nothing we can do
> with it and it's the price you pay for the optimizations, so again,
> I don't see a problem here.
I get what you mean with regard to getting the perfect situation, but
the kernel went 29 years without LD_ORPHAN_WARN. :) Anyway, we'll see
what other folks think, I guess.
> I still don't get why you're trying to split this series into two.
> It's been almost a year since v5 was published, I doubt you can get
> "basic FG-KASLR" accepted quickly just because it was reviewed back
Well, because it was blocked then by a single bug, and everything else
you've described are distinct improvements on v5, so to me it makes
sense to have it separated into those phases. I don't mean split the
series, I mean rearrange the series so that a rebased v5 is at the
start, and the improvements follow.
> I prefer to provide a full picture of what I'm trying to bring, so
> the community could review it all and throw much more ideas and
Understood. I am suggesting some ideas about how it might help with
> > > * It's now fully compatible with ClangLTO, ClangCFI,
> > > CONFIG_LD_ORPHAN_WARN and some more stuff landed since the last
> > > revision was published;
> > FWIW, v5 was was too. :) I didn't have to do anything to v5 to make it
> > work with ClangLTO and ClangCFI.
> Once again, repeating the thing I wrote earlier in our discussion:
> ClangCFI, at least shadowed implementation, requires the first text
> section of the module to be page-aligned and contain __cfi_check()
> at the very beginning of this section. With FG-KASLR and without
> special handling, this section gets randomized along with the
> others, and ClangCFI either rejects almost all modules or panics
> the kernel.
Ah-ha, thanks. I must have missed your answer to this earlier. I had
probably done my initial v5 testing without modules.
> > Great, this is a good start. One place we saw problems in the past was
> > with i386 build gotchas, so that'll need testing too.
> For now, FG_KASLR for x86 depends on X86_64. We might relax this
> dependency later after enough testing or whatsoever (like it's been
> done for ClangLTO).
Yes, but we've had a history of making big patches that do _intend_ to
break the i386 build, but they do anyway. Hence my question.
> > Sounds good. It might be easier to base the series on linux-next, so a
> > smaller series. Though given the merge window just opened, it might make
> > more sense for a v7 to be based on v5.15-rc2 in three weeks.
> I don't usually base any series on linux-next, because it contains
> all the changes from all "for-next" branches and repos, while the
> series finally gets accepted to the specific repo based on just
> v5.x-rc1 (sometimes on -rc2). This may bring additional apply/merge
Understood. I just find it confusing to include patches on lkml that
already exist in a -next branch. Perhaps base on kbuild -next?
> > > Kees Cook (2):
> > > x86/boot: Allow a "silent" kaslr random byte fetch
> > > x86/boot/compressed: Avoid duplicate malloc() implementations
> > These two can get landed right away -- they're standalone fixes that
> > can safely go in -tip.
> > >
> > > Kristen Carlson Accardi (9):
> > > x86: tools/relocs: Support >64K section headers
> > Same for this.
> They make little to no sense for non-FG-KASLR systems. And none of
> them are "pure" fixes.
> The same could be said about e.g. ORC lookup patch, but again, it
> makes no sense right now.
*shrug* They're trivial changes that have been reviewed before, so it
seems like we can avoid resending them every time.
> > I suspect it'll still be easier to review this series as a rebase v5
> > followed by the evolutionary improvements, since the "basic FGKASLR" has
> > been reviewed in the past, and is fairly noninvasive. The changes for
> > ASM, new .text rules, etc, make a lot more changes that I think would be
> > nice to have separate so reasonable a/b testing can be done.
> I don't see a point in testing it two times instead of just one, as
> well as in delivering this feature in two halves. It sounds like
> "let's introduce ClangLTO, but firstly only for modules, as LTO for
> vmlinux requires changes in objtool code and a special handling for
> the initcalls".
> The changes you mentioned only seem invasive, in fact, they can
> carry way less harm than the "basic FG-KASLR" itself.
Mostly it's a question of building on prior testing (v5 worked), so that
new changes can be debugged if they cause problems. Regardless, it's
been so long, perhaps it won't matter to other reviewers and they'll
want to just start over from scratch.
next prev parent reply other threads:[~2021-09-02 1:37 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-31 14:40 Alexander Lobakin
2021-08-31 14:40 ` [PATCH v6 kspp-next 01/22] kbuild: Fix TRIM_UNUSED_KSYMS with LTO_CLANG Alexander Lobakin
2021-08-31 14:40 ` [PATCH v6 kspp-next 02/22] kbuild: merge vmlinux_link() between the ordinary link and Clang LTO Alexander Lobakin
2021-08-31 14:40 ` [PATCH v6 kspp-next 03/22] kbuild: do not remove 'linux' link in scripts/link-vmlinux.sh Alexander Lobakin
2021-08-31 14:40 ` [PATCH v6 kspp-next 04/22] kbuild: merge vmlinux_link() between ARCH=um and other architectures Alexander Lobakin
2021-08-31 14:40 ` [PATCH v6 kspp-next 05/22] x86: tools/relocs: Support >64K section headers Alexander Lobakin
2021-08-31 14:40 ` [PATCH v6 kspp-next 06/22] x86/boot: Allow a "silent" kaslr random byte fetch Alexander Lobakin
2021-08-31 14:40 ` [PATCH v6 kspp-next 07/22] x86: Makefile: Add build and config option for CONFIG_FG_KASLR Alexander Lobakin
2021-08-31 14:41 ` [PATCH v6 kspp-next 08/22] Make sure ORC lookup covers the entire _etext - _stext Alexander Lobakin
2021-08-31 14:41 ` [PATCH v6 kspp-next 09/22] x86/tools: Add relative relocs for randomized functions Alexander Lobakin
2021-08-31 14:41 ` [PATCH v6 kspp-next 10/22] x86/boot/compressed: Avoid duplicate malloc() implementations Alexander Lobakin
2021-08-31 14:41 ` [PATCH v6 kspp-next 11/22] x86: Add support for function granular KASLR Alexander Lobakin
2021-08-31 14:41 ` [PATCH v6 kspp-next 12/22] linkage: add macros for putting ASM functions into own sections Alexander Lobakin
2021-08-31 14:41 ` [PATCH v6 kspp-next 13/22] x86: conditionally place regular ASM functions into separate sections Alexander Lobakin
2021-08-31 14:41 ` [PATCH v6 kspp-next 14/22] FG-KASLR: use a scripted approach to handle .text.* sections Alexander Lobakin
2021-08-31 14:41 ` [PATCH v6 kspp-next 15/22] kallsyms: Hide layout Alexander Lobakin
2021-08-31 14:41 ` [PATCH v6 kspp-next 16/22] livepatch: only match unique symbols when using fgkaslr Alexander Lobakin
2021-09-09 11:53 ` Miroslav Benes
2021-09-10 12:29 ` Alexander Lobakin
2021-08-31 14:41 ` [PATCH v6 kspp-next 17/22] x86/boot: allow FG-KASLR to be selected Alexander Lobakin
2021-08-31 14:41 ` [PATCH v6 kspp-next 18/22] arm64/crypto: conditionally place ASM functions into separate sections Alexander Lobakin
2021-08-31 14:41 ` [PATCH v6 kspp-next 19/22] module: Reorder functions Alexander Lobakin
2021-08-31 14:41 ` [PATCH v6 kspp-next 20/22] module: use a scripted approach for FG-KASLR Alexander Lobakin
2021-08-31 14:41 ` [PATCH v6 kspp-next 21/22] Documentation: add a documentation " Alexander Lobakin
2021-08-31 14:41 ` [PATCH v6 kspp-next 22/22] maintainers: add MAINTAINERS entry " Alexander Lobakin
2021-08-31 17:27 ` [PATCH v6 kspp-next 00/22] Function Granular KASLR Kees Cook
2021-09-01 10:36 ` Alexander Lobakin
2021-09-02 1:36 ` Kees Cook [this message]
2021-09-03 11:19 ` Alexander Lobakin
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:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--subject='Re: [PATCH v6 kspp-next 00/22] Function Granular KASLR' \
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).