[V3] xen: Check if the range is valid in init_domheap_pages

Message ID 1384349870-4256-1-git-send-email-julien.grall@linaro.org
State Accepted
Commit 45ffa79447954e68892322d985e72620d32ac16d
Headers show

Commit Message

Julien Grall Nov. 13, 2013, 1:37 p.m.
On ARM, when an initrd is given to xen by U-boot, it will reserve the memory in
the device tree.
In this case, when xen decides to free unused memory, dt_unreserved_regions
will call init_domheap_pages with the start and the end of range equals. But
the latter assumes that (start > end), if not Xen will hang because the
number of pages is equals to (unsigned)-1.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <jbeulich@suse.com>

---
    Changes in v3:
        - Invert the check

    Changes in v2:
        - Change commit title
        - Move the check in init_domheap_pages
---
 xen/common/page_alloc.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Keir Fraser Nov. 13, 2013, 2:58 p.m. | #1
On 13/11/2013 13:37, "Julien Grall" <julien.grall@linaro.org> wrote:

> On ARM, when an initrd is given to xen by U-boot, it will reserve the memory
> in
> the device tree.
> In this case, when xen decides to free unused memory, dt_unreserved_regions
> will call init_domheap_pages with the start and the end of range equals. But
> the latter assumes that (start > end), if not Xen will hang because the
> number of pages is equals to (unsigned)-1.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <jbeulich@suse.com>

I understand Jan's reservations, but I think just being more liberal here is
fine really.

Acked-by: Keir Fraser <keir@xen.org>
Julien Grall Nov. 20, 2013, 4:09 p.m. | #2
On 11/13/2013 01:37 PM, Julien Grall wrote:
> On ARM, when an initrd is given to xen by U-boot, it will reserve the memory in
> the device tree.
> In this case, when xen decides to free unused memory, dt_unreserved_regions
> will call init_domheap_pages with the start and the end of range equals. But
> the latter assumes that (start > end), if not Xen will hang because the
> number of pages is equals to (unsigned)-1.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <jbeulich@suse.com>

Ping ? Without this commit, the user is unable to pass initrd to dom0 on
ARM.
Keir Fraser Nov. 20, 2013, 9:12 p.m. | #3
On 20/11/2013 08:09, "Julien Grall" <julien.grall@citrix.com> wrote:

> On 11/13/2013 01:37 PM, Julien Grall wrote:
>> On ARM, when an initrd is given to xen by U-boot, it will reserve the memory
>> in
>> the device tree.
>> In this case, when xen decides to free unused memory, dt_unreserved_regions
>> will call init_domheap_pages with the start and the end of range equals. But
>> the latter assumes that (start > end), if not Xen will hang because the
>> number of pages is equals to (unsigned)-1.
>> 
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> CC: Keir Fraser <keir@xen.org>
>> CC: Jan Beulich <jbeulich@suse.com>
> 
> Ping ? Without this commit, the user is unable to pass initrd to dom0 on
> ARM.

It was fine with me.

Acked-by: Keir Fraser <keir@xen.org>

Patch

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 4c17fbd..0701333 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1429,6 +1429,9 @@  void init_domheap_pages(paddr_t ps, paddr_t pe)
     smfn = round_pgup(ps) >> PAGE_SHIFT;
     emfn = round_pgdown(pe) >> PAGE_SHIFT;
 
+    if ( emfn <= smfn )
+        return;
+
     init_heap_pages(mfn_to_page(smfn), emfn - smfn);
 }