Re: [PATCH v10 3/3] ARM: uncompress: Validate start of physical memory against passed DTB

[Geert Uytterhoeven]

Hi Russell,

On Thu, Dec 3, 2020 at 3:05 PM Russell King - ARM Linux admin
<linux@xxxxxxxxxxxxxxx> wrote:
> On Thu, Dec 03, 2020 at 01:19:16PM +0100, Geert Uytterhoeven wrote:
> > diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> > index d9cce7238a365081..1b6425df87e84e71 100644
> > --- a/arch/arm/boot/compressed/head.S
> > +++ b/arch/arm/boot/compressed/head.S
> > @@ -282,10 +282,36 @@ not_angel:
> >                * are already placing their zImage in (eg) the top 64MB
> >                * of this range.
> >                */
> > -             mov     r4, pc
> > -             and     r4, r4, #0xf8000000
> > +             mov     r0, pc
> > +             and     r0, r0, #0xf8000000
> > +#ifdef CONFIG_USE_OF
> > +             adr     r1, LC1
> > +#ifdef CONFIG_ARM_APPENDED_DTB
> > +             /*
> > +              * Look for an appended DTB.  If found, we cannot use it to
> > +              * validate the calculated start of physical memory, as its
> > +              * memory nodes may need to be augmented by ATAGS stored at
> > +              * an offset from the same start of physical memory.
> > +              */
> > +             ldr     r2, [r1, #4]    @ get &_edata
> > +             add     r2, r2, r1      @ relocate it
> > +             ldr     r2, [r2]        @ get DTB signature
> > +             ldr     r3, =OF_DT_MAGIC
> > +             cmp     r2, r3          @ do we have a DTB there?
> > +             beq     1f              @ if yes, skip validation
> > +#endif /* CONFIG_ARM_APPENDED_DTB */
> > +
> > +             /* Make sure we have some stack */
> > +             ldr     sp, [r1]        @ get stack location
> > +             add     sp, sp, r1      @ apply relocation
> > +
> > +             /* Validate calculated start against passed DTB */
> > +             mov     r1, r8
> > +             bl      fdt_check_mem_start
>
> I don't think this is going to work. You can only run C code when
> it has been linked for a specific address and is loaded at the
> correct address as it contains absolute addresses. We work around
> that in the decompressor by (ab)using the GOT table, and the code
> that fixes up the GOT table is run later, after you've called out
> to some C code here.
>
> Quite how this works for you without the GOT fixups having been
> done, I'm not sure.

It works in the same way as the existing atags_to_fdt() works:

     * [...] No GOT fixup has occurred
     * yet, but none of the code we're about to call uses any
     * global variable.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds