Re: [PATCH] RFC: build-sys: drop dtc submodule

[David Gibson]

On Thu, Aug 26, 2021 at 11:34:59AM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Aug 26, 2021 at 7:11 AM David Gibson <david@gibson.dropbear.id.au>
> wrote:
> 
> > On Thu, Aug 26, 2021 at 12:11:17AM +0400, Marc-André Lureau wrote:
> > > Hi
> > >
> > > On Thu, Aug 26, 2021 at 12:00 AM Peter Maydell <peter.maydell@linaro.org
> > >
> > > wrote:
> > >
> > > > On Wed, 25 Aug 2021 at 20:55, Marc-André Lureau
> > > > <marcandre.lureau@gmail.com> wrote:
> > > > > fdt_check_full was added in 1.4.7:
> > > > > https://git.kernel.org/pub/scm/utils/dtc/dtc.git/tag/?h=v1.4.7
> > > > >
> > > > > Only ubuntu appears to be lagging a bit behind. I wonder if they
> > would
> > > > consider an update.
> > > >
> > > > I doubt it. You would need to wait until that actually falls off
> > > > our supported list. You also have a couple of years to wait until
> > > > Debian oldstable is no longer on our supported list.
> > > >
> > > > Maybe, I don't know why debian oldstable would have received a new
> > version
> > > plus fixes, and not ubuntu.
> > >
> > > It seems we could have our own fallback copy of fdt_check_full() though..
> > > I'll give that a try.
> >
> > We could, but fdt_check_full() is actually a pretty complex function.
> >
> >
> Yeah, that would be used for those who don't have >= 1.4.7.
> 
> Alternatively we could lower the fdt_check_full to fdt_check_header in this
> case? It seems it is used to verify the DT from SLOF. It may be trusted I
> suppose, or a malformed DT may only impact the guest?

No, fdt_check_header() isn't enough.  We can't trust the dt blob from
SLOF, because it's coming from guest context.  We *expect* it to come
from the SLOF iamge we control, but nothing prevents anything else in
the guest from calling the hypercall, or corrupting the in-memory SLOF
image.

And, a bad DT won't just impact the guest - there's a couple of things
we need from it (that's the only reason we need to have SLOF give us
back the DT at all).  Note that the blob might not just have bad
content, but could have bad formatting which will make the functions
qemu uses to access it misbehave.  So, our options are either be
super-careful on every possible DT access after this point, or
pre-check it when it's loaded with fdt_check_full().

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature