Re: [Xen-devel] [PATCH 14/16] arm: load dom0 kernel from first boot module

[Tim Deegan <tim@xxxxxxx>]

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.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel