Re: [RFC PATCH v2 01/16] hvmloader: Fix scratch_alloc to avoid overlaps

[Ian Campbell <ian.campbell@citrix.com>]

On Mon, 2015-10-26 at 16:03 +0000, Anthony PERARD wrote:

Please remember to CC the relevant maintainers.

> scratch_alloc() set scratch_start to the last byte of the current
> allocation.  The value of scratch_start is then reused as is (if it is
> already aligned) in the next allocation.  This result in a potential reuse
> of the last byte of the previous allocation.

This will, in one obscure case, now result in the BUG_ON triggering when it
wouldn't before. Namely in the case where e was previously MAX_INT and is
now 0.

I'm not sure we care, since the end result previously would be that the
next scratch allocation came from address 0, which seems unlikely to end
well. So the only case we might actually care would be if the allocation
triggering the BUG_ON was the last one.

I think we probably don't care about this obscure corner case, especially
given that the allocations are starting from 0x00010000 and using enough
scratch to wrap seems like an indication the world has gone wrong anyway.

Maybe you want to discuss some of this in an updated commit message.

> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

(NB: hvmloader is formally maintained by the X86 ARCHITECTURE maintainers,
not the tools ones)

> ---
>  tools/firmware/hvmloader/util.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/firmware/hvmloader/util.c
> b/tools/firmware/hvmloader/util.c
> index d779fd7..42e8af4 100644
> --- a/tools/firmware/hvmloader/util.c
> +++ b/tools/firmware/hvmloader/util.c
> @@ -479,7 +479,7 @@ void *scratch_alloc(uint32_t size, uint32_t align)
>          align = 16;
>  
>      s = (scratch_start + align - 1) & ~(align - 1);
> -    e = s + size - 1;
> +    e = s + size;
>  
>      BUG_ON(e < s);
>  

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