Re: [PATCH v7 00/22] XSA55 libelf fixes for unstable

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 12/06/13 14:45, Ian Jackson wrote:
> Andrew Cooper writes ("Re: [PATCH v7 00/22] XSA55 libelf fixes for unstable"):
>> On 11/06/13 19:20, Ian Jackson wrote:
>>> This is version 7 of my series to try to fix libelf and the domain
>>> loader.
>> Fantastic.  This entire series is now
>>
>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Great, thanks.  The 4.1 backports are a work in progress.
>
> I have had a review from Oracle by private email.  Prompted by that
> review I did a search and now I'm suspicious of a range test in
> xc_dom_alloc_segment.
>
> I'm considering adding the patch below to the end of my series.
>
> Ian.
>
>
> Subject: libxc: Better range check in xc_dom_alloc_segment
>
> If seg->pfn is too large, the arithmetic in the range check might
> overflow, defeating the range check.
>
> This is part of the fix to a security issue, XSA-55.
>
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

Wow - yes that does need fixing.  It is the same sort of argument as for
the extra changes to patch 22.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

>
> diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c
> index bf05d2a..9c0a52c 100644
> --- a/tools/libxc/xc_dom_core.c
> +++ b/tools/libxc/xc_dom_core.c
> @@ -510,7 +510,8 @@ int xc_dom_alloc_segment(struct xc_dom_image *dom,
>      seg->vstart = start;
>      seg->pfn = (seg->vstart - dom->parms.virt_base) / page_size;
>  
> -    if ( pages > dom->total_pages || /* double test avoids overflow probs */
> +    if ( pages > dom->total_pages || /* multiple test avoids overflow probs */
> +         seg->pfn > dom->total_pages ||
>           pages > dom->total_pages - seg->pfn)
>      {
>          xc_dom_panic(dom->xch, XC_OUT_OF_MEMORY,