Re: [Xen-devel] [PATCH] xen/grant-table: correctly initialize grant table version 1

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2012-12-19 at 20:20 +0000, Matt Wilson wrote:
> Commit 85ff6acb075a484780b3d763fdf41596d8fc0970 (xen/granttable: Grant
> tables V2 implementation) changed the GREFS_PER_GRANT_FRAME macro from
> a constant to a conditional expression. The expression depends on
> grant_table_version being appropriately set. Unfortunately, at init
> time grant_table_version will be 0. The GREFS_PER_GRANT_FRAME
> conditional expression checks for "grant_table_version == 1", and
> therefore returns the number of grant references per frame for v2.
> 
> This causes gnttab_init() to allocate fewer pages for gnttab_list, as
> a frame can old half the number of v2 entries than v1 entries. After
> gnttab_resume() is called, grant_table_version is appropriately
> set. nr_init_grefs will then be miscalculated and gnttab_free_count
> will hold a value larger than the actual number of free gref entries.
> 
> If a guest is heavily utilizing improperly initialized v1 grant
> tables, memory corruption can occur.

Good catch!

[...]
@@ -1080,10 +1081,9 @@ static void gnttab_request_version(void)
>               panic("we need grant tables version 2, but only version 1 is 
> available");
>       } else {
>               grant_table_version = 1;
> +             grefs_per_grant_frame = PAGE_SIZE / sizeof(struct 
> grant_entry_v1);
>               gnttab_interface = &gnttab_v1_ops;
>       }
> -     printk(KERN_INFO "Grant tables using version %d layout.\n",
> -             grant_table_version);

You remove this here, and re-add it in the gnttab_resume callsite but I
don't  see anything added at the gnttab_init callsite. It would be
useful to keep this print at start of day too.

Oh, I see, gnttab_init also calls gnttab_resume later so we get the
message from there, with gnttab_request_version getting called twice.

Can we avoid doing this twice by moving the tail of gnttab_resume
(everything except the gnttab_request_version) into a new gnttab_setup
and calling that from gnttab_resume and gnttab_init (instead of calling
resume)?

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel