Re: 4e78921ba4dd ("efi/x86/Add missing error handling to old_memmap 1:1 mapping code")

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Guenter Roeck <groeck@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Zubin Mithra <zsm@chromium.org>, stable <stable@vger.kernel.org>,
	Guenter Roeck <groeck@chromium.org>,
	Gen Zhang <blackgod016574@gmail.com>,
	Darren Hart <dvhart@infradead.org>,
	Andy Shevchenko <andy@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>
Subject: Re: 4e78921ba4dd ("efi/x86/Add missing error handling to old_memmap 1:1 mapping code")
Date: Tue, 4 Jun 2019 11:03:25 +0200	[thread overview]
Message-ID: <CAKv+Gu-L-XGV8VkqDYzMZQeqQV2gePd7=texrneU0Ds8KJ9H+A@mail.gmail.com> (raw)
In-Reply-To: <CABXOdTeLtgjzL_V5rgsLnwZLaiK+MnL1BfOr8XeGXW8+Ws9zQQ@mail.gmail.com>

On Tue, 4 Jun 2019 at 10:52, Guenter Roeck <groeck@google.com> wrote:
>
> On Tue, Jun 4, 2019 at 12:46 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Jun 04, 2019 at 09:38:27AM +0200, Ard Biesheuvel wrote:
> > > On Tue, 4 Jun 2019 at 00:38, Zubin Mithra <zsm@chromium.org> wrote:
> > > >
> > > > Hello,
> > > >
> > > > CVE-2019-12380 was fixed in the upstream linux kernel with the commit :-
> > > > * 4e78921ba4dd ("efi/x86/Add missing error handling to old_memmap 1:1 mapping code")
> > > >
> > > > Could the patch be applied in order to v4.19.y?
> > > >
> > > > Tests run:
> > > > * Chrome OS tryjob
> > > >
> > >
> > > Unless I am missing something, it seems to me that there is some
> > > inflation going on when it comes to CVE number assignments.
> > >
> > > The code in question only affects systems that are explicitly booted
> > > with efi=old_map, and the memory allocation occurs so early during the
> > > boot sequence that even if we fail and handle it gracefully, it is
> > > highly unlikely that we can get to a point where the system is usable
> > > at all.
> > >
> > > Does Chrome OS boot in EFI mode? Does it use efi=old_map? Is the
> > > kernel built with 5 level paging enabled? Did you run it on 5 level
> > > paging hardware?
> > >
> > > Or is this just a tick the box exercise?
> > >
> > > Also, I am annoyed (does it show? :-))  that nobody mentioned the CVE
> > > at any point when the patch was under review (not even privately)
> >
> > CVEs are almost always asked for _after_ the patch is merged, as the
> > average fix-to-CVE request timeframe is -100 days.
> >
> > Also, for the kernel, CVEs almost mean nothing, so if this really isn't
> > an issue, I'll not backport this.
> >
> > And I really doubt that any chromeos device has 5 level page tables just
> > yet :)
> >
>
> FWIW, Chrome OS kernels are not only used in Chromebooks nowadays.
> They are also used in VM images in systems with hundreds of GB of
> memory. At least some of those may well boot in EFI mode.

Yes, but why would you boot those with efi=old_map, which is an option
that is only there for compatibility with old and non-standard EFI
implementations.

> Plus, as
> also mentioned, we do not (and will not) double-guess CVEs. If anyone
> has an issue with CVE creation, I would suggest to discuss with the
> respective bodies, not with us.
>

Fair enough.

> Zubin, as mentioned before, please hold back on -stable backport
> requests for CVE fixes. Please apply CVE fixes to our branches
> directly instead, per the above guidance ("for the kernel, CVEs almost
> mean nothing"). I'll revise our policy accordingly. Again, sorry for
> the trouble.
>

No trouble at all, and apologies for the grumpy tone.

In this particular case, the CVE is highly dubious (imo), since not
every bug is a vulnerability, and this bug is very difficult to hit
even on systems which make use of efi=old_map. While that also reduces
the risk of regressions, pulling this bug into a stable release
requires justification, and sadly, given the apparent policy issues
with assigning CVE numbers, the fact that the patch addresses a CVE is
not sufficient.