[PATCH 1/1] arm: do not copy magic 4 bytes of appended DTB in zImage

[PATCH 1/1] arm: do not copy magic 4 bytes of appended DTB in zImage

[Alexander Egorenkov]

If the passed zImage happens to have a DTB appended, then the magic 4 bytes
of the DTB are copied together with the kernel image. This leads to
failed kexec boots because the decompressor finds the aforementioned
DTB magic and falsely tries to replace the DTB passed in the register r2
with the non-existent appended one.

Signed-off-by: Alexander Egorenkov <egorenar-dev@posteo.net>
---
 kexec/arch/arm/kexec-zImage-arm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kexec/arch/arm/kexec-zImage-arm.c b/kexec/arch/arm/kexec-zImage-arm.c
index 925a9be..8ec289d 100644
--- a/kexec/arch/arm/kexec-zImage-arm.c
+++ b/kexec/arch/arm/kexec-zImage-arm.c
@@ -382,6 +382,7 @@ int zImage_arm_load(int argc, char **argv, const char *buf, off_t len,
 	unsigned int atag_offset = 0x1000; /* 4k offset from memory start */
 	unsigned int extra_size = 0x8000; /* TEXT_OFFSET */
 	const struct zimage_tag *tag;
+	size_t kernel_buf_size;
 	size_t kernel_mem_size;
 	const char *command_line;
 	char *modified_cmdline = NULL;
@@ -537,6 +538,8 @@ int zImage_arm_load(int argc, char **argv, const char *buf, off_t len,
 		}
 	}
 
+	kernel_buf_size = len;
+
 	/*
 	 * Always extend the zImage by four bytes to ensure that an appended
 	 * DTB image always sees an initialised value after _edata.
@@ -759,7 +762,7 @@ int zImage_arm_load(int argc, char **argv, const char *buf, off_t len,
 		add_segment(info, dtb_buf, dtb_length, dtb_offset, dtb_length);
 	}
 
-	add_segment(info, buf, len, kernel_base, kernel_mem_size);
+	add_segment(info, buf, kernel_buf_size, kernel_base, kernel_mem_size);
 
 	info->entry = (void*)kernel_base;
 
-- 
2.31.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 1/1] arm: do not copy magic 4 bytes of appended DTB in zImage

[Simon Horman]

On Thu, Apr 08, 2021 at 10:06:44PM +0200, Alexander Egorenkov wrote:
> If the passed zImage happens to have a DTB appended, then the magic 4 bytes
> of the DTB are copied together with the kernel image. This leads to
> failed kexec boots because the decompressor finds the aforementioned
> DTB magic and falsely tries to replace the DTB passed in the register r2
> with the non-existent appended one.
> 
> Signed-off-by: Alexander Egorenkov <egorenar-dev@posteo.net>

Hi,

I also see that, on line 558 len is further expanded as follows:

        /*
         * The zImage length does not include its stack (4k) or its
         * malloc space (64k).  Include this.
         */
        len += 0x11000;

Is it intentional that this patch also excludes this extra length
from the DTB? Or am I missing something?

> ---
>  kexec/arch/arm/kexec-zImage-arm.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/kexec/arch/arm/kexec-zImage-arm.c b/kexec/arch/arm/kexec-zImage-arm.c
> index 925a9be..8ec289d 100644
> --- a/kexec/arch/arm/kexec-zImage-arm.c
> +++ b/kexec/arch/arm/kexec-zImage-arm.c
> @@ -382,6 +382,7 @@ int zImage_arm_load(int argc, char **argv, const char *buf, off_t len,
>  	unsigned int atag_offset = 0x1000; /* 4k offset from memory start */
>  	unsigned int extra_size = 0x8000; /* TEXT_OFFSET */
>  	const struct zimage_tag *tag;
> +	size_t kernel_buf_size;
>  	size_t kernel_mem_size;
>  	const char *command_line;
>  	char *modified_cmdline = NULL;
> @@ -537,6 +538,8 @@ int zImage_arm_load(int argc, char **argv, const char *buf, off_t len,
>  		}
>  	}
>  
> +	kernel_buf_size = len;
> +
>  	/*
>  	 * Always extend the zImage by four bytes to ensure that an appended
>  	 * DTB image always sees an initialised value after _edata.
> @@ -759,7 +762,7 @@ int zImage_arm_load(int argc, char **argv, const char *buf, off_t len,
>  		add_segment(info, dtb_buf, dtb_length, dtb_offset, dtb_length);
>  	}
>  
> -	add_segment(info, buf, len, kernel_base, kernel_mem_size);
> +	add_segment(info, buf, kernel_buf_size, kernel_base, kernel_mem_size);
>  
>  	info->entry = (void*)kernel_base;
>  
> -- 
> 2.31.1
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 1/1] arm: do not copy magic 4 bytes of appended DTB in zImage

[Russell King - ARM Linux admin]

On Thu, Apr 08, 2021 at 10:06:44PM +0200, Alexander Egorenkov wrote:
> If the passed zImage happens to have a DTB appended, then the magic 4 bytes
> of the DTB are copied together with the kernel image. This leads to
> failed kexec boots because the decompressor finds the aforementioned
> DTB magic and falsely tries to replace the DTB passed in the register r2
> with the non-existent appended one.
> 
> Signed-off-by: Alexander Egorenkov <egorenar-dev@posteo.net>

This looks correct, but I would like a comment before the assignment
of kernel_buf_size to explain that this is the size of the compressed
kernel image excluding any appended DTB.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 1/1] arm: do not copy magic 4 bytes of appended DTB in zImage

[Alexander Egorenkov]

Simon Horman <horms@verge.net.au> writes:

> On Thu, Apr 08, 2021 at 10:06:44PM +0200, Alexander Egorenkov wrote:
>> If the passed zImage happens to have a DTB appended, then the magic 4 bytes
>> of the DTB are copied together with the kernel image. This leads to
>> failed kexec boots because the decompressor finds the aforementioned
>> DTB magic and falsely tries to replace the DTB passed in the register r2
>> with the non-existent appended one.
>> 
>> Signed-off-by: Alexander Egorenkov <egorenar-dev@posteo.net>
>
> Hi,
>
> I also see that, on line 558 len is further expanded as follows:
>
>         /*
>          * The zImage length does not include its stack (4k) or its
>          * malloc space (64k).  Include this.
>          */
>         len += 0x11000;
>
> Is it intentional that this patch also excludes this extra length
> from the DTB? Or am I missing something?
>

Hi,

if i understood it right, then len expresses not the length of the
kernel image in the zImage but the length of the kernel memory segment
into which the kernel image is being loaded. And on this line of code
it is adjusted to account for stack and heap, i think.

Regards
Alex

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 1/1] arm: do not copy magic 4 bytes of appended DTB in zImage

[Simon Horman]

On Mon, Apr 12, 2021 at 01:27:27PM +0200, Alexander Egorenkov wrote:
> Simon Horman <horms@verge.net.au> writes:
> 
> > On Thu, Apr 08, 2021 at 10:06:44PM +0200, Alexander Egorenkov wrote:
> >> If the passed zImage happens to have a DTB appended, then the magic 4 bytes
> >> of the DTB are copied together with the kernel image. This leads to
> >> failed kexec boots because the decompressor finds the aforementioned
> >> DTB magic and falsely tries to replace the DTB passed in the register r2
> >> with the non-existent appended one.
> >> 
> >> Signed-off-by: Alexander Egorenkov <egorenar-dev@posteo.net>
> >
> > Hi,
> >
> > I also see that, on line 558 len is further expanded as follows:
> >
> >         /*
> >          * The zImage length does not include its stack (4k) or its
> >          * malloc space (64k).  Include this.
> >          */
> >         len += 0x11000;
> >
> > Is it intentional that this patch also excludes this extra length
> > from the DTB? Or am I missing something?
> >
> 
> Hi,
> 
> if i understood it right, then len expresses not the length of the
> kernel image in the zImage but the length of the kernel memory segment
> into which the kernel image is being loaded. And on this line of code
> it is adjusted to account for stack and heap, i think.

Thanks, I think that answers my question.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec