From mboxrd@z Thu Jan  1 00:00:00 1970
From: Tim Deegan <tim@xen.org>
Subject: Re: [PATCH 14/16] arm: load dom0 kernel from first boot
	module
Date: Thu, 6 Sep 2012 14:44:27 +0100
Message-ID: <20120906134427.GN56463@ocelot.phlegethon.org>
References: <1346678886.32462.9.camel@zakaz.uk.xensource.com>
	<1346679056-8108-14-git-send-email-ian.campbell@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Content-Disposition: inline
In-Reply-To: <1346679056-8108-14-git-send-email-ian.campbell@citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Ian Campbell <ian.campbell@citrix.com>
Cc: xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

At 13:30 +0000 on 03 Sep (1346679054), Ian Campbell wrote:
> -static int kernel_try_zimage_prepare(struct kernel_info *info)
> +static int kernel_try_zimage_prepare(struct kernel_info *info,
> +                                     paddr_t addr, paddr_t size)
>  {
>      uint32_t *zimage = (void *)FIXMAP_ADDR(FIXMAP_MISC);
>      uint32_t start, end;
>      struct minimal_dtb_header dtb_hdr;
>  
> -    set_fixmap(FIXMAP_MISC, KERNEL_FLASH_ADDRESS >> PAGE_SHIFT, DEV_SHARED);
> +    set_fixmap(FIXMAP_MISC, addr >> PAGE_SHIFT, DEV_SHARED);
> +
> +    zimage += addr & ~PAGE_MASK;
>  
>      if (zimage[ZIMAGE_MAGIC_OFFSET/4] != ZIMAGE_MAGIC)
>          return -EINVAL;
> @@ -106,16 +109,24 @@ static int kernel_try_zimage_prepare(struct kernel_info *info)
>      start = zimage[ZIMAGE_START_OFFSET/4];
>      end = zimage[ZIMAGE_END_OFFSET/4];
>  
> +    if ( end > addr + size )
> +        return -EINVAL;
> +
>      clear_fixmap(FIXMAP_MISC);

No clear_fixmap() on the error path?  I see there isn't one on the
existing error path above, but I suspect that's not deliberate.

>  
>      /*
>       * Check for an appended DTB.
>       */
> -    copy_from_paddr(&dtb_hdr, KERNEL_FLASH_ADDRESS + end - start, sizeof(dtb_hdr), DEV_SHARED);
> +    copy_from_paddr(&dtb_hdr, addr + end - start, sizeof(dtb_hdr), DEV_SHARED);
>      if (be32_to_cpu(dtb_hdr.magic) == DTB_MAGIC) {
>          end += be32_to_cpu(dtb_hdr.total_size);
> +
> +        if ( end > addr + size )
> +            return -EINVAL;

There ought to be a bounds check before the copy_from_paddr as well
(though I suppose there's not much to do except fail more gracefully).

Tim.