Re: [PATCH for-4.6] tools/libxc: arm: Check the index before accessing the bank

[Julien Grall <julien.grall@citrix.com>]

On 17/09/15 18:36, Julien Grall wrote:
> When creating a guest with more than 3GB of memory, the 2 banks will be
> used and the loop with overrunning. The code will fail later on because
> Xen will deny to populate the region:
> 
> domainbuilder: detail: xc_dom_devicetree_mem: called
> domainbuilder: detail: xc_dom_mem_init: mem 3096 MB, pages 0xc1800 pages, 4k each
> domainbuilder: detail: xc_dom_mem_init: 0xc1800 pages
> domainbuilder: detail: xc_dom_boot_mem_init: called
> domainbuilder: detail: set_mode: guest xen-3.0-aarch64, address size 64
> domainbuilder: detail: xc_dom_malloc            : 14384 kB
> domainbuilder: detail: populate_guest_memory: populating RAM @0000000040000000-0000000100000000 (3072MB)
> domainbuilder: detail: populate_one_size: populated 0x3/0x3 entries with shift 18
> domainbuilder: detail: populate_guest_memory: populating RAM @0000000200000000-0000000201800000 (24MB)
> domainbuilder: detail: populate_one_size: populated 0xc/0xc entries with shift 9
> domainbuilder: detail: populate_guest_memory: populating RAM @0000007fad41c000-0007fb39dd42c000 (2141954816MB)
> domainbuilder: detail: populate_one_size: populated 0x100/0x1e4 entries with shift 0
> domainbuilder: detail: populate_guest_memory: Not enough RAM
> 
> This is because we are currently accessing the bank before checking the
> validity of the index. AFAICT, on  Debian Jessie, the compiler (gcc 4.9.2) is
> assuming that it's not necessary to verify the index because it's used
> before. This is a valid assumption because the operand of && are
> execute from from left to right.
> 
> Re-order the checks to verify the validity of the index before accessing
> the bank.
> 
> The problem has been present since the introduction of the multi-bank
> feature in commit 45d9867837f099e9eed4189dac5ed39d1fe2ed49 " tools: arm:
> prepare domain builder for multiple banks of guest RAM".

Hmmmm I forgot my Signed-off-by :(.

Signed-off-by: Julien Grall <julien.grall@citrix.com>

> ---
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> This patch is a candidate for Xen 4.6 and backport to Xen 4.5. Without
> it, it's impossible to boot guest using more than 3GB (limit after which
> the memory bank is used).
> ---
>  tools/libxc/xc_dom_arm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
> index b00d667..aeaba54 100644
> --- a/tools/libxc/xc_dom_arm.c
> +++ b/tools/libxc/xc_dom_arm.c
> @@ -460,7 +460,7 @@ int arch_setup_meminit(struct xc_dom_image *dom)
>          dom->p2m_host[pfn] = INVALID_P2M_ENTRY;
>  
>      /* setup initial p2m and allocate guest memory */
> -    for ( i = 0; dom->rambank_size[i] && i < GUEST_RAM_BANKS; i++ )
> +    for ( i = 0; i < GUEST_RAM_BANKS && dom->rambank_size[i]; i++ )
>      {
>          if ((rc = populate_guest_memory(dom,
>                                          bankbase[i] >> XC_PAGE_SHIFT,
> 


-- 
Julien Grall