Re: [Xen-devel] [PATCH v3] xen/tools: Introduce QNX IFS loader

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2014-09-23 at 19:24 +0300, Oleksandr Tyshchenko wrote:

> > BTW, you might want to check > dom->kernel_size to allow for smaller
> > images?
> No, I would prefer the strong check:
> if ( total_size != dom->kernel_size )
>    ...

OK, assuming you are sure that those two things always add up to the
total (what about startup_size?)

> >
> >> +    {
> >> +        xc_dom_printf(dom->xch, "%s: QNX IFS has wrong size", 
> >> __FUNCTION__);
> >> +        return -EINVAL;
> >> +    }
> >> +
> >> +    /* Performs a checksums on the startup and the OS image filesystem */
> >> +    if ( (calc_checksum((uint32_t *)startup_hdr, 
> >> startup_hdr->startup_size) != 0) ||
> >> +         (calc_checksum((uint32_t *)startup_hdr + 
> >> startup_hdr->startup_size/4,
> >> +          startup_hdr->stored_size - startup_hdr->startup_size) != 0) )
> >
> > You haven't validated startup_size yet, so you can't trust it to not
> > overrun the buffer. And you need to be careful with that subtraction,
> > probably starting with validating that one is larger than the other.
> ok
> 
> >
> > You don't see to use preboot_size for anything, perhaps you meant to
> > range check startup_size above instead?
> Sorry I don't understand what do you mean.

I mean that you validate preboot_size but then don't use it, while you
use startup_size without validating it. I was wondering if perhaps you
were accidentally checking the wrong one.

Ian.


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