All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tony Lindgren <tony@atomide.com>
To: Nicolas Pitre <nicolas.pitre@linaro.org>
Cc: Shawn Guo <shawn.guo@freescale.com>,
	linux-arm-kernel@lists.infradead.org, patches@linaro.org,
	Aaro Koskinen <aaro.koskinen@nokia.com>,
	linux-omap@vger.kernel.org
Subject: Re: [PATCH] ARM: Fix relocation if image end past uncompressed kernel end
Date: Thu, 28 Apr 2011 09:38:08 +0300	[thread overview]
Message-ID: <20110428063808.GK16892@atomide.com> (raw)
In-Reply-To: <alpine.LFD.2.00.1104271810450.24613@xanadu.home>

* 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

WARNING: multiple messages have this Message-ID (diff)
From: tony@atomide.com (Tony Lindgren)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: Fix relocation if image end past uncompressed kernel end
Date: Thu, 28 Apr 2011 09:38:08 +0300	[thread overview]
Message-ID: <20110428063808.GK16892@atomide.com> (raw)
In-Reply-To: <alpine.LFD.2.00.1104271810450.24613@xanadu.home>

* 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

  reply	other threads:[~2011-04-28  6:38 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-20  4:15 [PATCH 0/3] fix some issues with the kernel decompressor Nicolas Pitre
2011-04-20  4:15 ` [PATCH 1/3] ARM: zImage: no need to get the decompressed size from the filesystem Nicolas Pitre
2011-04-21 12:47   ` Tony Lindgren
2011-04-20  4:15 ` [PATCH 2/3] ARM: compressed/misc.c: simplify decompress_kernel() Nicolas Pitre
2011-04-21 12:49   ` Tony Lindgren
2011-04-27  8:53   ` Russell King - ARM Linux
2011-04-27 14:48     ` Nicolas Pitre
2011-04-28  8:12       ` Russell King - ARM Linux
2011-04-28  8:17       ` Tony Lindgren
2011-04-28 18:20         ` Nicolas Pitre
2011-04-29  7:17           ` Tony Lindgren
2011-04-20  4:15 ` [PATCH 3/3] ARM: zImage: fix issues with missing GOT entries for some global variables Nicolas Pitre
2011-04-20  8:42   ` Uwe Kleine-König
2011-04-20  9:19     ` Shawn Guo
2011-04-20 12:28     ` Nicolas Pitre
2011-04-20 12:36       ` Uwe Kleine-König
2011-04-20 12:47         ` Nicolas Pitre
2011-04-27  8:58           ` Russell King - ARM Linux
2011-04-27 15:00             ` Nicolas Pitre
2011-04-20 12:52       ` Shawn Guo
2011-04-20 13:03         ` Nicolas Pitre
2011-04-20 13:17           ` Nicolas Pitre
2011-04-21 12:50   ` Tony Lindgren
2011-04-27  8:55   ` Russell King - ARM Linux
2011-04-27 14:55     ` Nicolas Pitre
2011-04-20  7:21 ` [PATCH 0/3] fix some issues with the kernel decompressor Tony Lindgren
2011-04-20 13:00   ` Nicolas Pitre
2011-04-20 16:55     ` Tony Lindgren
2011-04-20 17:19       ` Nicolas Pitre
2011-04-21  5:59         ` Tony Lindgren
2011-04-21 10:49           ` [PATCH] ARM: Fix relocation if image end past uncompressed kernel end Tony Lindgren
2011-04-21 10:49             ` Tony Lindgren
2011-04-21 13:22             ` Nicolas Pitre
2011-04-21 13:22               ` Nicolas Pitre
2011-04-21 21:26               ` Nicolas Pitre
2011-04-21 21:26                 ` Nicolas Pitre
2011-04-22  3:23                 ` Nicolas Pitre
2011-04-22  3:23                   ` Nicolas Pitre
2011-04-22  5:19                   ` Shawn Guo
2011-04-22  5:19                     ` Shawn Guo
2011-04-22  5:36                     ` Shawn Guo
2011-04-22  5:36                       ` Shawn Guo
2011-04-22  6:28                   ` Tony Lindgren
2011-04-22  6:28                     ` Tony Lindgren
2011-04-22 14:12                     ` Nicolas Pitre
2011-04-22 14:12                       ` Nicolas Pitre
2011-04-26  8:57                       ` Tony Lindgren
2011-04-26  8:57                         ` Tony Lindgren
2011-04-26 12:37                         ` [PATCH] ARM: Fix bad SP address after relocating kernel Tony Lindgren
2011-04-26 12:37                           ` Tony Lindgren
2011-04-26 21:31                           ` Nicolas Pitre
2011-04-26 21:31                             ` Nicolas Pitre
2011-04-27  7:48                             ` Tony Lindgren
2011-04-27  7:48                               ` Tony Lindgren
2011-04-22  6:09               ` [PATCH] ARM: Fix relocation if image end past uncompressed kernel end Tony Lindgren
2011-04-22  6:09                 ` Tony Lindgren
2011-04-27 12:47               ` Tony Lindgren
2011-04-27 12:47                 ` Tony Lindgren
2011-04-27 12:56                 ` Tony Lindgren
2011-04-27 12:56                   ` Tony Lindgren
2011-04-27 22:16                   ` Nicolas Pitre
2011-04-27 22:16                     ` Nicolas Pitre
2011-04-28  6:38                     ` Tony Lindgren [this message]
2011-04-28  6:38                       ` Tony Lindgren
2011-04-28  8:12                       ` Tony Lindgren
2011-04-28  8:12                         ` Tony Lindgren
2011-04-20 11:16 ` [PATCH 0/3] fix some issues with the kernel decompressor Shawn Guo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110428063808.GK16892@atomide.com \
    --to=tony@atomide.com \
    --cc=aaro.koskinen@nokia.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=nicolas.pitre@linaro.org \
    --cc=patches@linaro.org \
    --cc=shawn.guo@freescale.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.