ARM: Fix relocation if image end past uncompressed kernel end

Message ID 20110421104954.GH13688@atomide.com
State New
Headers show

Commit Message

Tony Lindgren April 21, 2011, 10:49 a.m.
Otherwise we end up overwriting ourselves. This fixes booting
on n900 after commit 6d7d0ae51574943bf571d269da3243257a2d15db
(ARM: 6750/1: improvements to compressed/head.S).

Signed-off-by: Tony Lindgren <tony@atomide.com>

Comments

Nicolas Pitre April 21, 2011, 1:22 p.m. | #1
On Thu, 21 Apr 2011, Tony Lindgren wrote:

> Otherwise we end up overwriting ourselves. This fixes booting
> on n900 after commit 6d7d0ae51574943bf571d269da3243257a2d15db
> (ARM: 6750/1: improvements to compressed/head.S).
> 
> Signed-off-by: Tony Lindgren <tony@atomide.com>

I don't understand why this is needed.  The copy loop is explicitly 
copying from the end going backward exactly to cope with this 
possibility.

Hmmm...


Nicolas
Nicolas Pitre April 21, 2011, 9:26 p.m. | #2
On Thu, 21 Apr 2011, Nicolas Pitre wrote:

> On Thu, 21 Apr 2011, Tony Lindgren wrote:
> 
> > Otherwise we end up overwriting ourselves. This fixes booting
> > on n900 after commit 6d7d0ae51574943bf571d269da3243257a2d15db
> > (ARM: 6750/1: improvements to compressed/head.S).
> > 
> > Signed-off-by: Tony Lindgren <tony@atomide.com>
> 
> I don't understand why this is needed.  The copy loop is explicitly 
> copying from the end going backward exactly to cope with this 
> possibility.

I think your patch is 1) unneeded (see the copy loop code and the 
comment before it), and 2) simply hiding the real bug.

I just need to modify the code in compressed/misc.c slightly for the 
lzma decompressor to start or stop working randomly.  It seems that this 
code might be sensitive to slight displacement in memory caused by 
modifications to totally unrelated code.  I'm still trying to track this 
down.


Nicolas
Tony Lindgren April 22, 2011, 6:09 a.m. | #3
* Nicolas Pitre <nicolas.pitre@linaro.org> [110421 16:18]:
> On Thu, 21 Apr 2011, Tony Lindgren wrote:
> 
> > Otherwise we end up overwriting ourselves. This fixes booting
> > on n900 after commit 6d7d0ae51574943bf571d269da3243257a2d15db
> > (ARM: 6750/1: improvements to compressed/head.S).
> > 
> > Signed-off-by: Tony Lindgren <tony@atomide.com>
> 
> I don't understand why this is needed.  The copy loop is explicitly 
> copying from the end going backward exactly to cope with this 
> possibility.
> 
> Hmmm...

Yeah that's what I'm wondering too.. This is probably not the
right fix.. I'm also wondering that it should be possible to
make uImage also not work by setting loadaddr just before the
uncompressed kernel end.

You would assume that only the running code would not survive
relocation if some of it gets overwritten. But that should be
only the beginning, no idea why the need to relocate all the
way after the whole image?

If stack was overlapping the zImage, I could see it corrupt
the zImage but there not much happening between relocating
and restarting of the bootloader.

Tony

Patch

--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -282,6 +282,7 @@  dtb_check_done:
 
 /*
  * Check to see if we will overwrite ourselves.
+ *   r1  = corrupted, temporary uncompressed kernel end 
  *   r4  = final kernel address
  *   r5  = start of this image
  *   r9  = size of decompressed image
@@ -292,15 +293,24 @@  dtb_check_done:
  */
 		cmp	r4, r10
 		bhs	wont_overwrite
-		add	r10, r4, r9
-		cmp	r10, r5
+		add	r1, r4, r9
+		cmp	r1, r5
 		bls	wont_overwrite
 
+		/*
+		 * Check if the compressed image end is past the uncompressed
+		 * kernel end. In that case, relocate ourselves to the end
+		 * of the compressed image instead of the uncompressed kernel
+		 * end to avoid overwriting ourselves.
+		 */
+		cmp	r10, r1
+		movls	r10, r1
+
 /*
  * Relocate ourselves past the end of the decompressed kernel.
  *   r5  = start of this image
  *   r6  = _edata
- *   r10 = end of the decompressed kernel
+ *   r10 = end of the decompressed kernel or end of this image if larger
  * Because we always copy ahead, we need to do it from the end and go
  * backward in case the source and destination overlap.
  */