linux-kernel - Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 24 Oct 2017 10:30:41 +0100
From:   Ard Biesheuvel <ard.biesheuvel@...aro.org>
To:     Russell King - ARM Linux <linux@...linux.org.uk>
Cc:     Chris Zhong <chris.zhong@...k-chips.com>,
        jeffy <jeffy.chen@...k-chips.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        Ingo Molnar <mingo@...nel.org>
Subject: Re: [PATCH] ARM: Fix zImage file size not aligned with
 CONFIG_EFI_STUB enabled

On 24 October 2017 at 10:26, Ard Biesheuvel <ard.biesheuvel@...aro.org> wrote:
> On 24 October 2017 at 10:22, Russell King - ARM Linux
> <linux@...linux.org.uk> wrote:
>> On Tue, Oct 24, 2017 at 10:13:09AM +0100, Ard Biesheuvel wrote:
>>> On 24 October 2017 at 10:09, Russell King - ARM Linux
>>> <linux@...linux.org.uk> wrote:
>>> > The question is: do we want to know when additional sections get
>>> > emitted into the binary?
>>>
>>> Well, we need to know whether the size of zImage is a multiple of 512
>>> bytes. We could check that separately by adding the following as well
>>>
>>> #ifdef CONFIG_EFI_STUB
>>> ASSERT((_edata % 512 == 0), "zImage file size is not a multiple of 512 bytes")
>>
>> ASSERT((_edata - _text) % 512 == 0, "EFI zImage file size is not a multiple of 512 bytes")
>>
>> This would only catch them when EFI is enabled, which doesn't give very
>> good build coverage.  I still prefer my solution over adding the assert
>> and a section for _edata - my solution catches it with EFI disabled as
>> well.
>>
>> A variant on that would be this, which should catch additional sections
>> for EFI and non-EFI:
>>
>> diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
>> index b38dcef90756..9d0c5d80979a 100644
>> --- a/arch/arm/boot/compressed/vmlinux.lds.S
>> +++ b/arch/arm/boot/compressed/vmlinux.lds.S
>> @@ -95,6 +95,10 @@ SECTIONS
>>
>>    _edata = .;
>>
>> +  .image_end (NOLOAD) : {
>> +    _image_end = .;
>> +  }
>> +
>>    _magic_sig = ZIMAGE_MAGIC(0x016f2818);
>>    _magic_start = ZIMAGE_MAGIC(_start);
>>    _magic_end = ZIMAGE_MAGIC(_edata);
>> @@ -119,3 +123,5 @@ SECTIONS
>>    .stab.indexstr 0     : { *(.stab.indexstr) }
>>    .comment 0           : { *(.comment) }
>>  }
>> +
>> +ASSERT(image_end == end, "zImage file size is incorrect")
>>
>
> I take it you mean
>
> _image_end == _edata
>
> here? That works for me:
>
> Tested-by: Ard Biesheuvel <ard.biesheuvel@...aro.org>
>
> (with CONFIG_EFI enabled)


BTW you could simplify that to

diff --git a/arch/arm/boot/compressed/vmlinux.lds.S
b/arch/arm/boot/compressed/vmlinux.lds.S
index 7a4c59154361..204d4580a04f 100644
--- a/arch/arm/boot/compressed/vmlinux.lds.S
+++ b/arch/arm/boot/compressed/vmlinux.lds.S
@@ -85,6 +85,10 @@ SECTIONS

   _edata = .;

+  .image_end (NOLOAD) : {
+    ASSERT(. == _edata, "zImage file size is incorrect");
+  }
+
   _magic_sig = ZIMAGE_MAGIC(0x016f2818);
   _magic_start = ZIMAGE_MAGIC(_start);
   _magic_end = ZIMAGE_MAGIC(_edata);