Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190620003244.261595-1-ndesaulniers@google.com>
 <20190620074640.GA27228@brain-police> <CAKv+Gu_KCFCVxw_zAfzUf8DjD4DmhvaJEoqBsX_SigOse_NwYw@mail.gmail.com>
 <CAKwvOdmQ+WdD8nvLz_VB_5atDi56fv485Xsn+mHJZKnyj6L-JA@mail.gmail.com> <20190624095749.wasjfrgcda7ygdr5@willie-the-truck>
In-Reply-To: <20190624095749.wasjfrgcda7ygdr5@willie-the-truck>
From:   Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date:   Mon, 24 Jun 2019 12:06:18 +0200
Message-ID: <CAKv+Gu8G2GQGxmcAAy1XQ5gkN-2fJSWAKCQQm9T4skYdh5cT3Q@mail.gmail.com>
Subject: Re: [PATCH] arm64: defconfig: update and enable CONFIG_RANDOMIZE_BASE
To:     Will Deacon <will@kernel.org>
Cc:     Nick Desaulniers <ndesaulniers@google.com>,
        Kees Cook <keescook@google.com>,
        Sami Tolvanen <samitolvanen@google.com>,
        Jeffrey Vander Stoep <jeffv@google.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Enric Balletbo i Serra <enric.balletbo@collabora.com>,
        Arnd Bergmann <arnd@arndb.de>,
        Maxime Ripard <maxime.ripard@bootlin.com>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will.deacon@arm.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Dinh Nguyen <dinguyen@kernel.org>,
        Mark Brown <broonie@kernel.org>,
        Jagan Teki <jagan@amarulasolutions.com>,
        Olof Johansson <olof@lixom.net>,
        Shawn Guo <shawnguo@kernel.org>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, 24 Jun 2019 at 11:57, Will Deacon <will@kernel.org> wrote:
>
> Hi Nick, Kees, Ard,
>
> Thanks for the responses.
>
> On Fri, Jun 21, 2019 at 01:27:45PM -0700, Nick Desaulniers wrote:
> > On Thu, Jun 20, 2019 at 1:17 AM Ard Biesheuvel
> > <ard.biesheuvel@linaro.org> wrote:
> > > On Thu, 20 Jun 2019 at 09:47, Will Deacon <will.deacon@arm.com> wrote:
> > > > On the flip side, I worry that it could make debugging more difficult, but I
> > > > don't know whether that's a genuine concern or not. I'm assuming you've
> > > > debugged your fair share of crashes from KASLR-enabled kernels; how bad is
> > > > it? (I'm thinking of the case where somebody mails you part of a panic log
> > > > and a .config).
> >
> > I don't recall specific cases where KASLR made debugging difficult.  I
> > went and spoke to our stability team that debugs crash reports from
> > the field.  My understanding is that we capture full ramdumps.  They
> > have a lot of custom tooling for debugging, but they did not recall
> > ever having to disable KASLR to debug further.  We've had KASLR
> > enabled since I think the 2016 Pixel 1, so I assume their tooling
> > accounts for the seed/offset.
> >
> > I think if a full ramdump of the kernel image is loaded into GDB with
> > the matching kernel image it "just works" but could be mistaken.  For
> > external developers, "nokaslr" boot time param is pretty standard.
> >
> > > In fact, given how many Android phones are running this code: Nick,
> > > can you check if there are any KASLR related kernel fixes that haven't
> > > been upstreamed?
> >
> > I spoke with the android common kernel team that's trying to burn down
> > their out of tree patches.  I triple checked a doc they had where they
> > had audited every last patch, looking for for KASLR and
> > CONFIG_RANDOMIZE_BASE.  I also triple checked our internal bug tracker
> > for burning down the out of tree patches.  Finally I'm scanning each
> > branch of our android-common trees via `git log --all --grep
> > <KASLR|CONFIG_RANDOMIZE_BASE>`.  I haven't found anything yet, and the
> > team doesn't expect any out of tree patches related to that feature.
> > Sorry for not responding sooner, but I'm still going through our 4.4,
> > 4.9, 4.14, and 4.19 branches.
>
> Thanks for having a look. It could be that we've fixed the issue Catalin was
> running into in the past -- he was going to see if the problem persists with
> mainline, since it was frequent enough that it was causing us to ignore the
> results from our testing infrastructure when RANDOMIZE_BASE=y.
>

I had no idea this was the case. I can look into it if we are still
seeing failures.

> > > So KASLR is known to be broken unless you enable KPTI as well, so that
> > > is something we could take into account. I.e., mitigations that don't
> > > reduce the attack surface at all are just pointless complexity, which
> > > should obviously be avoided.
> >
> > (Note to Sami + Jeff if they had KPTI on their radar)
>
> I mean, we could have RANDOMIZE_BASE select UNMAP_KERNEL_AT_EL0 if you like?
> The latter is already default y and hidden behind EXPERT.
>

IIRC, when KASLR is enabled (and we have a seed), we override the
runtime decision to out out of KPTI, and so even uarchs that don't
require the Meltdown mitigations it provides will still be using it.

So I'd be fine with just adding a note to the UNMAP_KERNEL_AT_EL0
Kconfig help text that even non-affected uarchs have a use for it if
KASLR is enabled, but given that it is already behind EXPERT, I don't
think more hand holding is necessary.

> > > Another thing to note is that the runtime cost of KASLR is ~zero, with
> > > the exception of the module PLTs. However, the latter could do with
> > > some additional coverage as well, so in summary, I think enabling this
> > > is a good thing. Otherwise, we could disable full module randomization
> > > so that the module PLT code doesn't get used in practice.
> > >
> > > Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >
> > Olof mentioned on IRC that I should resend without the other defconfig
> > changes.  Do others have thoughts on that?
>
> That's not a bad idea. If you do that, feel free to add my Ack to the one
> adding RANDOMIZE_BASE=y:
>
> Acked-by: Will Deacon <will@kernel.org>
>
> Will