[PATCH v2] tools/libs/ctrl: fix dumping of ballooned guest

Juergen Gross posted 1 patch 3 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/20201111100143.13820-1-jgross@suse.com
Maintainers: Ian Jackson <iwj@xenproject.org>, Wei Liu <wl@xen.org>
tools/libs/ctrl/xc_core.c | 42 +++++++++++++++++++++++++++++----------
1 file changed, 31 insertions(+), 11 deletions(-)
[PATCH v2] tools/libs/ctrl: fix dumping of ballooned guest
Posted by Juergen Gross 3 years, 5 months ago
A guest with memory < maxmem often can't be dumped via xl dump-core
without an error message today:

xc: info: exceeded nr_pages (262144) losing pages

In case the last page of the guest isn't allocated the loop in
xc_domain_dumpcore_via_callback() will always spit out this message,
as the number of already dumped pages is tested before the next page
is checked to be valid.

The guest's p2m_size might be lower than expected, so this should be
tested in order to avoid reading past the end of it.

The guest might use high bits in p2m entries to flag special cases like
foreign mappings. Entries with an MFN larger than the highest MFN of
the host should be skipped.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/libs/ctrl/xc_core.c | 42 +++++++++++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/tools/libs/ctrl/xc_core.c b/tools/libs/ctrl/xc_core.c
index e8c6fb96f9..b47ab2f6d8 100644
--- a/tools/libs/ctrl/xc_core.c
+++ b/tools/libs/ctrl/xc_core.c
@@ -439,6 +439,7 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
     unsigned long i;
     unsigned long j;
     unsigned long nr_pages;
+    unsigned long max_mfn;
 
     xc_core_memory_map_t *memory_map = NULL;
     unsigned int nr_memory_map;
@@ -577,6 +578,10 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
                                    &p2m, &dinfo->p2m_size);
         if ( sts != 0 )
             goto out;
+
+        sts = xc_maximum_ram_page(xch, &max_mfn);
+        if ( sts != 0 )
+            goto out;
     }
     else
     {
@@ -818,19 +823,12 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
         {
             uint64_t gmfn;
             void *vaddr;
-            
-            if ( j >= nr_pages )
-            {
-                /*
-                 * When live dump-mode (-L option) is specified,
-                 * guest domain may increase memory.
-                 */
-                IPRINTF("exceeded nr_pages (%ld) losing pages", nr_pages);
-                goto copy_done;
-            }
 
             if ( !auto_translated_physmap )
             {
+                if ( i >= dinfo->p2m_size )
+                    break;
+
                 if ( dinfo->guest_width >= sizeof(unsigned long) )
                 {
                     if ( dinfo->guest_width == sizeof(unsigned long) )
@@ -846,6 +844,14 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
                     if ( gmfn == (uint32_t)INVALID_PFN )
                        continue;
                 }
+                if ( gmfn > max_mfn )
+                    continue;
+
+                if ( j >= nr_pages )
+                {
+                    j++;
+                    continue;
+                }
 
                 p2m_array[j].pfn = i;
                 p2m_array[j].gmfn = gmfn;
@@ -855,6 +861,12 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
                 if ( !xc_core_arch_gpfn_may_present(&arch_ctxt, i) )
                     continue;
 
+                if ( j >= nr_pages )
+                {
+                    j++;
+                    continue;
+                }
+
                 gmfn = i;
                 pfn_array[j] = i;
             }
@@ -879,7 +891,15 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
         }
     }
 
-copy_done:
+    if ( j > nr_pages )
+    {
+        /*
+         * When live dump-mode (-L option) is specified,
+         * guest domain may increase memory.
+         */
+        IPRINTF("exceeded nr_pages (%ld) losing %ld pages", nr_pages, j - nr_pages);
+    }
+
     sts = dump_rtn(xch, args, dump_mem_start, dump_mem - dump_mem_start);
     if ( sts != 0 )
         goto out;
-- 
2.26.2


Re: [PATCH v2] tools/libs/ctrl: fix dumping of ballooned guest
Posted by Jürgen Groß 3 years, 5 months ago
On 11.11.20 11:01, Juergen Gross wrote:
> A guest with memory < maxmem often can't be dumped via xl dump-core
> without an error message today:
> 
> xc: info: exceeded nr_pages (262144) losing pages
> 
> In case the last page of the guest isn't allocated the loop in
> xc_domain_dumpcore_via_callback() will always spit out this message,
> as the number of already dumped pages is tested before the next page
> is checked to be valid.
> 
> The guest's p2m_size might be lower than expected, so this should be
> tested in order to avoid reading past the end of it.
> 
> The guest might use high bits in p2m entries to flag special cases like
> foreign mappings. Entries with an MFN larger than the highest MFN of
> the host should be skipped.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

This is a real bug fix.

Can any maintainer please have a look?


Juergen

> ---
>   tools/libs/ctrl/xc_core.c | 42 +++++++++++++++++++++++++++++----------
>   1 file changed, 31 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/libs/ctrl/xc_core.c b/tools/libs/ctrl/xc_core.c
> index e8c6fb96f9..b47ab2f6d8 100644
> --- a/tools/libs/ctrl/xc_core.c
> +++ b/tools/libs/ctrl/xc_core.c
> @@ -439,6 +439,7 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
>       unsigned long i;
>       unsigned long j;
>       unsigned long nr_pages;
> +    unsigned long max_mfn;
>   
>       xc_core_memory_map_t *memory_map = NULL;
>       unsigned int nr_memory_map;
> @@ -577,6 +578,10 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
>                                      &p2m, &dinfo->p2m_size);
>           if ( sts != 0 )
>               goto out;
> +
> +        sts = xc_maximum_ram_page(xch, &max_mfn);
> +        if ( sts != 0 )
> +            goto out;
>       }
>       else
>       {
> @@ -818,19 +823,12 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
>           {
>               uint64_t gmfn;
>               void *vaddr;
> -
> -            if ( j >= nr_pages )
> -            {
> -                /*
> -                 * When live dump-mode (-L option) is specified,
> -                 * guest domain may increase memory.
> -                 */
> -                IPRINTF("exceeded nr_pages (%ld) losing pages", nr_pages);
> -                goto copy_done;
> -            }
>   
>               if ( !auto_translated_physmap )
>               {
> +                if ( i >= dinfo->p2m_size )
> +                    break;
> +
>                   if ( dinfo->guest_width >= sizeof(unsigned long) )
>                   {
>                       if ( dinfo->guest_width == sizeof(unsigned long) )
> @@ -846,6 +844,14 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
>                       if ( gmfn == (uint32_t)INVALID_PFN )
>                          continue;
>                   }
> +                if ( gmfn > max_mfn )
> +                    continue;
> +
> +                if ( j >= nr_pages )
> +                {
> +                    j++;
> +                    continue;
> +                }
>   
>                   p2m_array[j].pfn = i;
>                   p2m_array[j].gmfn = gmfn;
> @@ -855,6 +861,12 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
>                   if ( !xc_core_arch_gpfn_may_present(&arch_ctxt, i) )
>                       continue;
>   
> +                if ( j >= nr_pages )
> +                {
> +                    j++;
> +                    continue;
> +                }
> +
>                   gmfn = i;
>                   pfn_array[j] = i;
>               }
> @@ -879,7 +891,15 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
>           }
>       }
>   
> -copy_done:
> +    if ( j > nr_pages )
> +    {
> +        /*
> +         * When live dump-mode (-L option) is specified,
> +         * guest domain may increase memory.
> +         */
> +        IPRINTF("exceeded nr_pages (%ld) losing %ld pages", nr_pages, j - nr_pages);
> +    }
> +
>       sts = dump_rtn(xch, args, dump_mem_start, dump_mem - dump_mem_start);
>       if ( sts != 0 )
>           goto out;
> 

Re: [PATCH v2] tools/libs/ctrl: fix dumping of ballooned guest
Posted by Jürgen Groß 3 years, 4 months ago
On 20.11.20 16:33, Jürgen Groß wrote:
> On 11.11.20 11:01, Juergen Gross wrote:
>> A guest with memory < maxmem often can't be dumped via xl dump-core
>> without an error message today:
>>
>> xc: info: exceeded nr_pages (262144) losing pages
>>
>> In case the last page of the guest isn't allocated the loop in
>> xc_domain_dumpcore_via_callback() will always spit out this message,
>> as the number of already dumped pages is tested before the next page
>> is checked to be valid.
>>
>> The guest's p2m_size might be lower than expected, so this should be
>> tested in order to avoid reading past the end of it.
>>
>> The guest might use high bits in p2m entries to flag special cases like
>> foreign mappings. Entries with an MFN larger than the highest MFN of
>> the host should be skipped.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> This is a real bug fix.
> 
> Can any maintainer please have a look?

PING?


Juergen

> 
> 
> Juergen
> 
>> ---
>>   tools/libs/ctrl/xc_core.c | 42 +++++++++++++++++++++++++++++----------
>>   1 file changed, 31 insertions(+), 11 deletions(-)
>>
>> diff --git a/tools/libs/ctrl/xc_core.c b/tools/libs/ctrl/xc_core.c
>> index e8c6fb96f9..b47ab2f6d8 100644
>> --- a/tools/libs/ctrl/xc_core.c
>> +++ b/tools/libs/ctrl/xc_core.c
>> @@ -439,6 +439,7 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
>>       unsigned long i;
>>       unsigned long j;
>>       unsigned long nr_pages;
>> +    unsigned long max_mfn;
>>       xc_core_memory_map_t *memory_map = NULL;
>>       unsigned int nr_memory_map;
>> @@ -577,6 +578,10 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
>>                                      &p2m, &dinfo->p2m_size);
>>           if ( sts != 0 )
>>               goto out;
>> +
>> +        sts = xc_maximum_ram_page(xch, &max_mfn);
>> +        if ( sts != 0 )
>> +            goto out;
>>       }
>>       else
>>       {
>> @@ -818,19 +823,12 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
>>           {
>>               uint64_t gmfn;
>>               void *vaddr;
>> -
>> -            if ( j >= nr_pages )
>> -            {
>> -                /*
>> -                 * When live dump-mode (-L option) is specified,
>> -                 * guest domain may increase memory.
>> -                 */
>> -                IPRINTF("exceeded nr_pages (%ld) losing pages", 
>> nr_pages);
>> -                goto copy_done;
>> -            }
>>               if ( !auto_translated_physmap )
>>               {
>> +                if ( i >= dinfo->p2m_size )
>> +                    break;
>> +
>>                   if ( dinfo->guest_width >= sizeof(unsigned long) )
>>                   {
>>                       if ( dinfo->guest_width == sizeof(unsigned long) )
>> @@ -846,6 +844,14 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
>>                       if ( gmfn == (uint32_t)INVALID_PFN )
>>                          continue;
>>                   }
>> +                if ( gmfn > max_mfn )
>> +                    continue;
>> +
>> +                if ( j >= nr_pages )
>> +                {
>> +                    j++;
>> +                    continue;
>> +                }
>>                   p2m_array[j].pfn = i;
>>                   p2m_array[j].gmfn = gmfn;
>> @@ -855,6 +861,12 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
>>                   if ( !xc_core_arch_gpfn_may_present(&arch_ctxt, i) )
>>                       continue;
>> +                if ( j >= nr_pages )
>> +                {
>> +                    j++;
>> +                    continue;
>> +                }
>> +
>>                   gmfn = i;
>>                   pfn_array[j] = i;
>>               }
>> @@ -879,7 +891,15 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
>>           }
>>       }
>> -copy_done:
>> +    if ( j > nr_pages )
>> +    {
>> +        /*
>> +         * When live dump-mode (-L option) is specified,
>> +         * guest domain may increase memory.
>> +         */
>> +        IPRINTF("exceeded nr_pages (%ld) losing %ld pages", nr_pages, 
>> j - nr_pages);
>> +    }
>> +
>>       sts = dump_rtn(xch, args, dump_mem_start, dump_mem - 
>> dump_mem_start);
>>       if ( sts != 0 )
>>           goto out;
>>
> 

Re: [PATCH v2] tools/libs/ctrl: fix dumping of ballooned guest
Posted by Wei Liu 3 years, 4 months ago
On Wed, Nov 11, 2020 at 11:01:43AM +0100, Juergen Gross wrote:
> A guest with memory < maxmem often can't be dumped via xl dump-core
> without an error message today:
> 
> xc: info: exceeded nr_pages (262144) losing pages
> 
> In case the last page of the guest isn't allocated the loop in
> xc_domain_dumpcore_via_callback() will always spit out this message,
> as the number of already dumped pages is tested before the next page
> is checked to be valid.
> 
> The guest's p2m_size might be lower than expected, so this should be
> tested in order to avoid reading past the end of it.
> 
> The guest might use high bits in p2m entries to flag special cases like
> foreign mappings. Entries with an MFN larger than the highest MFN of
> the host should be skipped.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked + applied.