diff mbox

ARM: Fix relocation if image end past uncompressed kernel end

Message ID alpine.LFD.2.00.1104271810450.24613@xanadu.home (mailing list archive)
State New, archived
Headers show

Commit Message

Nicolas Pitre April 27, 2011, 10:16 p.m. UTC
On Wed, 27 Apr 2011, Tony Lindgren wrote:

> * Tony Lindgren <tony@atomide.com> [110427 05:44]:
> > We can't overwrite the running code when relocating only a small amount,
> > say 0x100 or so.
> > 
> > There's no need to relocate all the way past the compressed kernel,
> > we just need to relocate past the size of the code in head.o.
> > 
> > Updated patch below using the GOT end instead of the compressed
> > image end.
> 
> Oops, the mov should be movle of course. Updated patch below.

This is wrong.  You're using r12 before it is fixed up with the 
proper offset.

And this could simply be fixed with a big enough constant like this:

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Tony Lindgren April 28, 2011, 6:38 a.m. UTC | #1
* Nicolas Pitre <nicolas.pitre@linaro.org> [110428 01:12]:
> On Wed, 27 Apr 2011, Tony Lindgren wrote:
> 
> > * Tony Lindgren <tony@atomide.com> [110427 05:44]:
> > > We can't overwrite the running code when relocating only a small amount,
> > > say 0x100 or so.
> > > 
> > > There's no need to relocate all the way past the compressed kernel,
> > > we just need to relocate past the size of the code in head.o.
> > > 
> > > Updated patch below using the GOT end instead of the compressed
> > > image end.
> > 
> > Oops, the mov should be movle of course. Updated patch below.
> 
> This is wrong.  You're using r12 before it is fixed up with the 
> proper offset.

Hmm I see. I guess I was thinking it only needs to be fixed up after
the relocation.
 
> And this could simply be fixed with a big enough constant like this:
> 
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index 8dab5e3..71fc1d9 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -250,8 +250,11 @@ restart:	adr	r0, LC0
>   * Because we always copy ahead, we need to do it from the end and go
>   * backward in case the source and destination overlap.
>   */
> -		/* Round up to next 256-byte boundary. */
> -		add	r10, r10, #256
> +		/*
> +		 * Round to a 256-byte boundary on the next page. This
> +		 * avoids overwriting ourself if the offset is small.
> +		 */
> +		add	r10, r10, #4096
>  		bic	r10, r10, #255
>  
>  		sub	r9, r6, r5		@ size to copy

Yeah that's what I had originally, but then we'll be potentially
hitting the same bug again once more cache flushing code etc gets
added.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 8dab5e3..71fc1d9 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -250,8 +250,11 @@  restart:	adr	r0, LC0
  * Because we always copy ahead, we need to do it from the end and go
  * backward in case the source and destination overlap.
  */
-		/* Round up to next 256-byte boundary. */
-		add	r10, r10, #256
+		/*
+		 * Round to a 256-byte boundary on the next page. This
+		 * avoids overwriting ourself if the offset is small.
+		 */
+		add	r10, r10, #4096
 		bic	r10, r10, #255
 
 		sub	r9, r6, r5		@ size to copy