Re: [Xen-devel] [PATCH] fix BUG in gnttab_unpopulate_status_frames()

[Jan Beulich <JBeulich@xxxxxxxx>]

On 30.07.2019 18:44, Paul Durrant wrote:
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -1682,6 +1682,14 @@ gnttab_unpopulate_status_frames(struct domain *d, 
> struct grant_table *gt)
>           struct page_info *pg = virt_to_page(gt->status[i]);
>           gfn_t gfn = gnttab_get_frame_gfn(gt, true, i);
>   
> +        if ( !get_page(pg, d) )
> +        {
> +            gprintk(XENLOG_ERR,
> +                    "Could not get a reference to status frame %u\n", i);
> +            domain_crash(d);
> +            return -EINVAL;
> +        }
> +
>           /*
>            * For translated domains, recovering from failure after partial
>            * changes were made is more complicated than it seems worth
> @@ -1708,6 +1716,7 @@ gnttab_unpopulate_status_frames(struct domain *d, 
> struct grant_table *gt)
>   
>           BUG_ON(page_get_owner(pg) != d);
>           put_page_alloc_ref(pg);
> +        put_page(pg);
>   
>           if ( pg->count_info & ~PGC_xen_heap )
>           {
> 

I dislike this approach, and not chosing the alternative of excluding
xenheap pages in the check in put_page_alloc_ref() (as I had recommended
elsewhere) should at least be discussed in the description. It is the
very nature of xenheap pages that they won't get freed, and hence don't
need this extra ref to be held for clearing PGC_allocated.

Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel