Re: [Xen-devel] [PATCH] do_memory_op: cleanup if copy_to_guest fails

[Keir Fraser <keir@xxxxxxx>]

On 16/12/2010 17:59, "Olaf Hering" <olaf@xxxxxxxxx> wrote:

> Undo the page allocation in the ulikely event the copy_to_guest fails.

You can't really clean up in this case. Once a page has been alloc'ed to a
domain, it can immediately see it and map it. Trying to then
free_domheap_page() ignoring the current page reference count is actually
introducing a bug.

Leaving a bit of a mess on failed copy_to_guest is okay imo, as that is
usually a pretty fatal sign anyway. If there were good reason for cleaning
up better, we should at least be doing if (test_and_clear(PGC_allocated))
put_page().

 -- Keir

> Signed-off-by: Olaf Hering <olaf@xxxxxxxxx>
> 
> ---
> 
> I have not exercised this code path, it was found during code inspection in
> 4.0
> 
>  xen/common/memory.c |    9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> --- xen-unstable.hg-4.1.22548.orig/xen/common/memory.c
> +++ xen-unstable.hg-4.1.22548/xen/common/memory.c
> @@ -82,7 +82,10 @@ static void increase_reservation(struct
>          {
>              mfn = page_to_mfn(page);
>              if ( unlikely(__copy_to_guest_offset(a->extent_list, i, &mfn, 1))
> )
> +            {
> +                free_domheap_pages(page, a->extent_order);
>                  goto out;
> +            }
>          }
>      }
>  
> @@ -144,7 +147,13 @@ static void populate_physmap(struct memo
>  
>                  /* Inform the domain of the new page's machine address. */
>                  if ( unlikely(__copy_to_guest_offset(a->extent_list, i, &mfn,
> 1)) )
> +                {
> +                    for ( j = 0; j < (1 << a->extent_order); j++ )
> +                        set_gpfn_from_mfn(mfn + j, INVALID_M2P_ENTRY);
> +                    guest_physmap_remove_page(d, gpfn, mfn, a->extent_order);
> +                    free_domheap_pages(page, a->extent_order);
>                      goto out;
> +                }
>              }
>          }
>      }
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel