On 09/08/18 11:41, Paul Durrant wrote: >> -----Original Message----- >> From: Andrew Cooper [mailto:[email protected]] >> Sent: 09 August 2018 11:32 >> To: Xen-devel <[email protected]> >> Cc: Andrew Cooper <[email protected]>; Jan Beulich >> <[email protected]>; Paul Durrant <[email protected]>; Stefano >> Stabellini <[email protected]>; Julien Grall <[email protected]>; Wei >> Liu <[email protected]>; Roger Pau Monne <[email protected]>; >> George Dunlap <[email protected]> >> Subject: [PATCH] common/gnttab: Explicitly default to gnttab v1 during >> domain creation >> >> For reasons which appear to be exclusively down to poor review of the grant >> table v2 code, a grant table's version field was wasn't initialised during >> creation. >> >> A number of problems (including XSAs) have occurred in the past trying >> trying >> to use a grant table which hasn't been properly set up, and various areas of >> the code cope with v0 by defaulting to v1. >> >> In particular, the toolstack using GNTTABOP_setup_table to be able to fill in >> the store/console grants has a side effect of switching to v1. >> >> In hindsight however, this "fixup if we see 0" is a very poor, with a >> substantial degree of risk. Explicitly default to grant table v1 during >> domain create, and let the rest of the code work safely in the knowledge >> that >> the version is sensibly set. >> >> Signed-off-by: Andrew Cooper <[email protected]> >> --- >> CC: Jan Beulich <[email protected]> >> CC: Paul Durrant <[email protected]> >> CC: Stefano Stabellini <[email protected]> >> CC: Julien Grall <[email protected]> >> CC: Wei Liu <[email protected]> >> CC: Roger Pau Monné <[email protected]> >> CC: George Dunlap <[email protected]> >> >> This probably wants backporting, so I chose not to integrate it into my >> series >> which rearranges large chunks of DOMCTL_createdomain >> --- >> xen/common/grant_table.c | 40 +++++++--------------------------------- >> 1 file changed, 7 insertions(+), 33 deletions(-) >> >> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c >> index d9ec711..8bae656 100644 >> --- a/xen/common/grant_table.c >> +++ b/xen/common/grant_table.c >> @@ -49,10 +49,7 @@ struct grant_table { >> percpu_rwlock_t lock; >> /* Lock protecting the maptrack limit */ >> spinlock_t maptrack_lock; >> - /* >> - * The defined versions are 1 and 2. Set to 0 if we don't know >> - * what version to use yet. >> - */ > Given that only the guest can set the version by hypercall, it might be worth > comment somewhere saying that grant table always start in v1 format and then > get morphed if the guest sets v2. I guess it might be inferred that a v1 > table is immutable (for those who don't look too hard).
Really? I don't think that's a reasonable inference to make, given GNTTABOP_set_version as a public interface. ~Andrew _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
