[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 1/3] gnttab: never permit mapping transitive grants
On 18.02.2021 11:25, Julien Grall wrote: > On 17/02/2021 10:46, Jan Beulich wrote: >> Transitive grants allow an intermediate domain I to grant a target >> domain T access to a page which origin domain O did grant I access to. >> As an implementation restriction, T is not allowed to map such a grant. >> This restriction is currently tried to be enforced by marking active >> entries resulting from transitive grants as is-sub-page; sub-page grants >> for obvious reasons don't allow mapping. However, marking (and checking) >> only active entries is insufficient, as a map attempt may also occur on >> a grant not otherwise in use. When not presently in use (pin count zero) >> the grant type itself needs checking. Otherwise T may be able to map an >> unrelated page owned by I. This is because the "transitive" sub- >> structure of the v2 union would end up being interpreted as "full_page" >> sub-structure instead. The low 32 bits of the GFN used would match the >> grant reference specified in I's transitive grant entry, while the upper >> 32 bits could be random (depending on how exactly I sets up its grant >> table entries). >> >> Note that if one mapping already exists and the granting domain _then_ >> changes the grant to GTF_transitive (which the domain is not supposed to >> do), the changed type will only be honored after the pin count has gone >> back to zero. This is no different from e.g. GTF_readonly or >> GTF_sub_page becoming set when a grant is already in use. >> >> While adjusting the implementation, also adjust commentary in the public >> header to better reflect reality. >> >> Fixes: 3672ce675c93 ("Transitive grant support") >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> > > The change in grant_table.c looks good to me: > > Acked-by: Julien Grall <jgrall@xxxxxxxxxx> Thanks. >> --- a/xen/include/public/grant_table.h >> +++ b/xen/include/public/grant_table.h >> @@ -166,11 +166,13 @@ typedef struct grant_entry_v1 grant_entr >> #define GTF_type_mask (3U<<0) >> >> /* >> - * Subflags for GTF_permit_access. >> + * Subflags for GTF_permit_access and GTF_transitive. >> * GTF_readonly: Restrict @domid to read-only mappings and accesses. [GST] >> * GTF_reading: Grant entry is currently mapped for reading by @domid. >> [XEN] >> * GTF_writing: Grant entry is currently mapped for writing by @domid. >> [XEN] >> - * GTF_PAT, GTF_PWT, GTF_PCD: (x86) cache attribute flags for the grant >> [GST] >> + * Further subflags for GTF_permit_access only. >> + * GTF_PAT, GTF_PWT, GTF_PCD: (x86) cache attribute flags to be used for >> + * mappings of the grant [GST] >> * GTF_sub_page: Grant access to only a subrange of the page. @domid >> * will only be allowed to copy from the grant, and not >> * map it. [GST] > > Do we have any check preventing GTF_sub_page and GTF_transitive to be > set together? No, and I also don't think we need one. For one transitive grants get implicitly treated as sub-page ones (I admit this is an implementation detail, not really an ABI aspect) and the flag is simply ignored in this case. Much like - see patch 3 - the flag ought to also be ignored for v1 grants. Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |