Re: [PATCH v2 00/29] implement KASLR for ARM
diff mbox

Message ID CAKv+Gu-HGM6T6cV25bo6_bBcZaV-x3qpc9XoYUA4AEx68Joz=A@mail.gmail.com
State New
Headers show

Commit Message

Ard Biesheuvel Sept. 6, 2017, 4:35 p.m. UTC
On 6 September 2017 at 17:31, Tony Lindgren <tony@atomide.com> wrote:
> * Ard Biesheuvel <ard.biesheuvel@linaro.org> [170906 09:26]:
>> On 6 September 2017 at 17:22, Tony Lindgren <tony@atomide.com> wrote:
>> > Sure was not able to reproduce it so far on BBB. But here's
>> > failed boot output from logicpd-torpedo-37xx-devkit. Will
>> > try some more booting on BBB too.
> ...
>> > 8< -------------------
>> > Kernel image @ 0x81000000 [ 0x000000 - 0x426810 ]
>> > ## Flattened Device Tree blob at 84000000
>> >    Booting using the fdt blob at 0x84000000
>> >    Loading Device Tree to 86feb000, end 86fff2d5 ... OK
>> >
>> > Starting kernel ...
>> >
>> > regions.image_size:00e00000
>> > regions.pa_start:80000000
>> > regions.pa_end:88000000
>> > regions.zimage_start:81000000
>> > regions.zimage_size:00437830
>> > regions.dtb_start:86feb000
>> > regions.dtb_size:00012000
>> > regions.initrd_start:00000000
>> > regions.initrd_size:00000000
>> > num:0000002f
>> > num:00000029
>> > *kaslr_offset:07400000
>> > Uncompressing Linux...
>>
>> Is that all? Does it hang while decompressing the kernel?
>
> Yeah so it seems. If we had uncompress overwriting something
> because of the increase in size it should happen on every
> boot though, not once every five boots or so.
>

Turns out I am calculating the top of DRAM incorrectly for boards
where less memory is present than the size of the lowmem region.

Could you try this please? (Apologies for the whitespace)

        regions.dtb_start = (u32)fdt;
@@ -391,7 +390,8 @@ u32 kaslr_early_init(u32 *kaslr_offset, u32
image_base, u32 image_size,
        }

        /* check the memory nodes for the size of the lowmem region */
-       regions.pa_end = min(regions.pa_end, get_memory_end(fdt));
+       regions.pa_end = min(regions.pa_end, get_memory_end(fdt)) -
+                        regions.image_size;

        puthex32(regions.image_size);
        puthex32(regions.pa_start);

Comments

Tony Lindgren Sept. 6, 2017, 5:12 p.m. UTC | #1
* Ard Biesheuvel <ard.biesheuvel@linaro.org> [170906 09:36]:
> Turns out I am calculating the top of DRAM incorrectly for boards
> where less memory is present than the size of the lowmem region.
> 
> Could you try this please? (Apologies for the whitespace)

I think for 10 or so units per year you can actually buy
a non-mangling outgoing SMTP service if nothing else helps..

Anyways, you patch manually applied fixed most of the random
boot hangs for me, but I did see a new one after 11 boot
attempts, see below.

Regards,

Tony

8< --------------
Kernel image @ 0x81000000 [ 0x000000 - 0x426300 ]
## Flattened Device Tree blob at 84000000
   Booting using the fdt blob at 0x84000000
   Loading Device Tree to 86feb000, end 86fff2d5 ... OK

Starting kernel ...

regions.image_size:00e00000
regions.pa_start:80000000
regions.pa_end:87200000
regions.zimage_start:81000000
regions.zimage_size:00437320
regions.dtb_start:86feb000
regions.dtb_size:00012000
regions.initrd_start:00000000
regions.initrd_size:00000000
num:00000028
num:00000025
*kaslr_offset:05e00000
Uncompressing Linux... done, booting the kernel.
Warning: Neither atags nor dtb found
Ard Biesheuvel Sept. 6, 2017, 5:30 p.m. UTC | #2
On 6 September 2017 at 18:12, Tony Lindgren <tony@atomide.com> wrote:
> * Ard Biesheuvel <ard.biesheuvel@linaro.org> [170906 09:36]:
>> Turns out I am calculating the top of DRAM incorrectly for boards
>> where less memory is present than the size of the lowmem region.
>>
>> Could you try this please? (Apologies for the whitespace)
>
> I think for 10 or so units per year you can actually buy
> a non-mangling outgoing SMTP service if nothing else helps..
>

Yeah, you're right, apologies. I use git-send-email mostly, but not
for inline snippets like this. And it is not actually the SMTP service
but the web client that mangles the whitespace

> Anyways, you patch manually applied fixed most of the random
> boot hangs for me, but I did see a new one after 11 boot
> attempts, see below.
>
> Regards,
>
> Tony
>
> 8< --------------
> Kernel image @ 0x81000000 [ 0x000000 - 0x426300 ]
> ## Flattened Device Tree blob at 84000000
>    Booting using the fdt blob at 0x84000000
>    Loading Device Tree to 86feb000, end 86fff2d5 ... OK
>
> Starting kernel ...
>
> regions.image_size:00e00000
> regions.pa_start:80000000
> regions.pa_end:87200000
> regions.zimage_start:81000000
> regions.zimage_size:00437320
> regions.dtb_start:86feb000
> regions.dtb_size:00012000
> regions.initrd_start:00000000
> regions.initrd_size:00000000
> num:00000028
> num:00000025
> *kaslr_offset:05e00000
> Uncompressing Linux... done, booting the kernel.
> Warning: Neither atags nor dtb found

OK, so in this case, 80000000 + 00e00000 + 05e00000 == 86c00000, which
is still below the DTB, but apparently, it has corrupted it anyway.

I will try to figure out what's going on here.

Thanks again for taking the time,
Ard.
Tony Lindgren Sept. 6, 2017, 5:53 p.m. UTC | #3
* Ard Biesheuvel <ard.biesheuvel@linaro.org> [170906 10:31]:
> On 6 September 2017 at 18:12, Tony Lindgren <tony@atomide.com> wrote:
> > Kernel image @ 0x81000000 [ 0x000000 - 0x426300 ]
> > ## Flattened Device Tree blob at 84000000
> >    Booting using the fdt blob at 0x84000000
> >    Loading Device Tree to 86feb000, end 86fff2d5 ... OK
> >
> > Starting kernel ...
> >
> > regions.image_size:00e00000
> > regions.pa_start:80000000
> > regions.pa_end:87200000
> > regions.zimage_start:81000000
> > regions.zimage_size:00437320
> > regions.dtb_start:86feb000
> > regions.dtb_size:00012000
> > regions.initrd_start:00000000
> > regions.initrd_size:00000000
> > num:00000028
> > num:00000025
> > *kaslr_offset:05e00000
> > Uncompressing Linux... done, booting the kernel.
> > Warning: Neither atags nor dtb found
> 
> OK, so in this case, 80000000 + 00e00000 + 05e00000 == 86c00000, which
> is still below the DTB, but apparently, it has corrupted it anyway.
> 
> I will try to figure out what's going on here.

Do you need to have kaslr_offset beyond the uncompressed
kernel size maybe?

> Thanks again for taking the time,

No problem, I'm happy to test these changes.

Regards,

Tony
Ard Biesheuvel Sept. 6, 2017, 6:04 p.m. UTC | #4
On 6 September 2017 at 18:53, Tony Lindgren <tony@atomide.com> wrote:
> * Ard Biesheuvel <ard.biesheuvel@linaro.org> [170906 10:31]:
>> On 6 September 2017 at 18:12, Tony Lindgren <tony@atomide.com> wrote:
>> > Kernel image @ 0x81000000 [ 0x000000 - 0x426300 ]
>> > ## Flattened Device Tree blob at 84000000
>> >    Booting using the fdt blob at 0x84000000
>> >    Loading Device Tree to 86feb000, end 86fff2d5 ... OK
>> >
>> > Starting kernel ...
>> >
>> > regions.image_size:00e00000
>> > regions.pa_start:80000000
>> > regions.pa_end:87200000
>> > regions.zimage_start:81000000
>> > regions.zimage_size:00437320
>> > regions.dtb_start:86feb000
>> > regions.dtb_size:00012000
>> > regions.initrd_start:00000000
>> > regions.initrd_size:00000000
>> > num:00000028
>> > num:00000025
>> > *kaslr_offset:05e00000
>> > Uncompressing Linux... done, booting the kernel.
>> > Warning: Neither atags nor dtb found
>>
>> OK, so in this case, 80000000 + 00e00000 + 05e00000 == 86c00000, which
>> is still below the DTB, but apparently, it has corrupted it anyway.
>>
>> I will try to figure out what's going on here.
>
> Do you need to have kaslr_offset beyond the uncompressed
> kernel size maybe?
>

I think the problem is in the rounding of region.pa_start.

I have now changed this to

regions.image_size = image_base % SZ_128M + round_up(image_size, SZ_2M);
regions.pa_start = round_down(image_base, SZ_128M);


>> Thanks again for taking the time,
>
> No problem, I'm happy to test these changes.
>

I have updated my arm-kaslr-v3 with all the fixes from this discussion
(and more)

Thanks,
Tony Lindgren Sept. 6, 2017, 6:22 p.m. UTC | #5
* Ard Biesheuvel <ard.biesheuvel@linaro.org> [170906 11:05]:
> On 6 September 2017 at 18:53, Tony Lindgren <tony@atomide.com> wrote:
> > * Ard Biesheuvel <ard.biesheuvel@linaro.org> [170906 10:31]:
> >> On 6 September 2017 at 18:12, Tony Lindgren <tony@atomide.com> wrote:
> >> > Kernel image @ 0x81000000 [ 0x000000 - 0x426300 ]
> >> > ## Flattened Device Tree blob at 84000000
> >> >    Booting using the fdt blob at 0x84000000
> >> >    Loading Device Tree to 86feb000, end 86fff2d5 ... OK
> >> >
> >> > Starting kernel ...
> >> >
> >> > regions.image_size:00e00000
> >> > regions.pa_start:80000000
> >> > regions.pa_end:87200000
> >> > regions.zimage_start:81000000
> >> > regions.zimage_size:00437320
> >> > regions.dtb_start:86feb000
> >> > regions.dtb_size:00012000
> >> > regions.initrd_start:00000000
> >> > regions.initrd_size:00000000
> >> > num:00000028
> >> > num:00000025
> >> > *kaslr_offset:05e00000
> >> > Uncompressing Linux... done, booting the kernel.
> >> > Warning: Neither atags nor dtb found
> >>
> >> OK, so in this case, 80000000 + 00e00000 + 05e00000 == 86c00000, which
> >> is still below the DTB, but apparently, it has corrupted it anyway.
> >>
> >> I will try to figure out what's going on here.
> >
> > Do you need to have kaslr_offset beyond the uncompressed
> > kernel size maybe?
> >
> 
> I think the problem is in the rounding of region.pa_start.
> 
> I have now changed this to
> 
> regions.image_size = image_base % SZ_128M + round_up(image_size, SZ_2M);
> regions.pa_start = round_down(image_base, SZ_128M);
...

> I have updated my arm-kaslr-v3 with all the fixes from this discussion
> (and more)

Looks like your branch at commit 5221c86ad2e7 still failed
after boot attempt #4, see below. Is that the right commit?

Regards,

Tony

8< -----------------
regions.zimage_size:004379e8
regions.dtb_start:86feb000
regions.dtb_size:00012000
regions.initrd_start:00000000
regions.initrd_size:00000000
count:00000028
num:00000025
*kaslr_offset:05e00000
Uncompressing Linux... done, booting the kernel.
Warning: Neither atags nor dtb found
Ard Biesheuvel Sept. 6, 2017, 6:25 p.m. UTC | #6
On 6 September 2017 at 19:22, Tony Lindgren <tony@atomide.com> wrote:
> * Ard Biesheuvel <ard.biesheuvel@linaro.org> [170906 11:05]:
>> On 6 September 2017 at 18:53, Tony Lindgren <tony@atomide.com> wrote:
>> > * Ard Biesheuvel <ard.biesheuvel@linaro.org> [170906 10:31]:
>> >> On 6 September 2017 at 18:12, Tony Lindgren <tony@atomide.com> wrote:
>> >> > Kernel image @ 0x81000000 [ 0x000000 - 0x426300 ]
>> >> > ## Flattened Device Tree blob at 84000000
>> >> >    Booting using the fdt blob at 0x84000000
>> >> >    Loading Device Tree to 86feb000, end 86fff2d5 ... OK
>> >> >
>> >> > Starting kernel ...
>> >> >
>> >> > regions.image_size:00e00000
>> >> > regions.pa_start:80000000
>> >> > regions.pa_end:87200000
>> >> > regions.zimage_start:81000000
>> >> > regions.zimage_size:00437320
>> >> > regions.dtb_start:86feb000
>> >> > regions.dtb_size:00012000
>> >> > regions.initrd_start:00000000
>> >> > regions.initrd_size:00000000
>> >> > num:00000028
>> >> > num:00000025
>> >> > *kaslr_offset:05e00000
>> >> > Uncompressing Linux... done, booting the kernel.
>> >> > Warning: Neither atags nor dtb found
>> >>
>> >> OK, so in this case, 80000000 + 00e00000 + 05e00000 == 86c00000, which
>> >> is still below the DTB, but apparently, it has corrupted it anyway.
>> >>
>> >> I will try to figure out what's going on here.
>> >
>> > Do you need to have kaslr_offset beyond the uncompressed
>> > kernel size maybe?
>> >
>>
>> I think the problem is in the rounding of region.pa_start.
>>
>> I have now changed this to
>>
>> regions.image_size = image_base % SZ_128M + round_up(image_size, SZ_2M);
>> regions.pa_start = round_down(image_base, SZ_128M);
> ...
>
>> I have updated my arm-kaslr-v3 with all the fixes from this discussion
>> (and more)
>
> Looks like your branch at commit 5221c86ad2e7 still failed
> after boot attempt #4, see below. Is that the right commit?
>

It should be, yes.

> 8< -----------------
> regions.zimage_size:004379e8
> regions.dtb_start:86feb000
> regions.dtb_size:00012000
> regions.initrd_start:00000000
> regions.initrd_size:00000000
> count:00000028
> num:00000025
> *kaslr_offset:05e00000
> Uncompressing Linux... done, booting the kernel.
> Warning: Neither atags nor dtb found

Did you capture the image_size and pa_start/pa_end as well?

In any case, this is the exact same offset that failed before, so the
rounding of pa_start wasn't the problem.
Tony Lindgren Sept. 6, 2017, 8:08 p.m. UTC | #7
* Ard Biesheuvel <ard.biesheuvel@linaro.org> [170906 11:25]:
> Did you capture the image_size and pa_start/pa_end as well?

Sorry I guess that was an incomplete copy & paste, but here's
another one:

regions.image_size:00e08000
regions.pa_start:80000000
regions.pa_end:871f8000
regions.zimage_start:81000000
regions.zimage_size:004379e8
regions.dtb_start:86feb000
regions.dtb_size:00012000
regions.initrd_start:00000000
regions.initrd_size:00000000
count:00000028
num:00000025
*kaslr_offset:05e00000
Uncompressing Linux... done, booting the kernel.
Warning: Neither atags nor dtb found

> In any case, this is the exact same offset that failed before, so the
> rounding of pa_start wasn't the problem.

OK

Regards,

Tony
Ard Biesheuvel Sept. 12, 2017, 6:51 a.m. UTC | #8
On 6 September 2017 at 21:08, Tony Lindgren <tony@atomide.com> wrote:
> * Ard Biesheuvel <ard.biesheuvel@linaro.org> [170906 11:25]:
>> Did you capture the image_size and pa_start/pa_end as well?
>
> Sorry I guess that was an incomplete copy & paste, but here's
> another one:
>
> regions.image_size:00e08000
> regions.pa_start:80000000
> regions.pa_end:871f8000
> regions.zimage_start:81000000
> regions.zimage_size:004379e8
> regions.dtb_start:86feb000
> regions.dtb_size:00012000
> regions.initrd_start:00000000
> regions.initrd_size:00000000
> count:00000028
> num:00000025
> *kaslr_offset:05e00000
> Uncompressing Linux... done, booting the kernel.
> Warning: Neither atags nor dtb found
>
>> In any case, this is the exact same offset that failed before, so the
>> rounding of pa_start wasn't the problem.
>
> OK
>

I am pretty sure this is an issue with .bss, which is not covered by
image_size (and which is absolutely *huge* btw [8+ MB] on
omap2plus_defconfig, due to lockdep being enabled by default)

Patch
diff mbox

diff --git a/arch/arm/boot/compressed/kaslr.c b/arch/arm/boot/compressed/kaslr.c
index d43c0be9b47b..e9c86809c857 100644
--- a/arch/arm/boot/compressed/kaslr.c
+++ b/arch/arm/boot/compressed/kaslr.c
@@ -339,8 +339,7 @@  u32 kaslr_early_init(u32 *kaslr_offset, u32
image_base, u32 image_size,

        regions.image_size = round_up(image_size, SZ_2M);
        regions.pa_start = round_down(image_base, SZ_128M);
-       regions.pa_end = lowmem_top - PAGE_OFFSET + regions.pa_start -
-                        regions.image_size;
+       regions.pa_end = lowmem_top - PAGE_OFFSET + regions.pa_start;
        regions.zimage_start = zimage_start;
        regions.zimage_size = zimage_end - zimage_start;