On 26/08/2021 11:15, Jan Beulich wrote:
> Returning back truncated frame numbers is unhelpful: Quite likely
> they're not owned by the domain (if it's PV), or we may misguide the
> guest into writing grant entries into a page that it actually uses for
> other purposes.
> Signed-off-by: Jan Beulich <jbeul...@suse.com>
> ---
> RFC: Arguably in the 32-bit PV case it may be necessary to instead put
> in place an explicit address restriction when allocating
> ->shared_raw[N]. This is currently implicit by alloc_xenheap_page()
> only returning memory covered by the direct-map.
Yet another reason why having the grant table be Xen memory, rather than
guest memory, was a terrible idea. Changing this is in consideration
for the encrypted vm work.
Its fine for now, but is fragile and liable to break for e.g. an
xmalloc() -> vmalloc() conversion, or when we get 5-level paging and the
directmap boundary moves above 16T.
> --- a/xen/common/compat/grant_table.c
> +++ b/xen/common/compat/grant_table.c
> @@ -175,8 +175,15 @@ int compat_grant_table_op(unsigned int c
> i < (_s_)->nr_frames; ++i ) \
> { \
> compat_pfn_t frame = (_s_)->frame_list.p[i]; \
> - if ( __copy_to_compat_offset((_d_)->frame_list, \
> - i, &frame, 1) ) \
> + if ( frame != (_s_)->frame_list.p[i] ) \
> + { \
> + if ( VALID_M2P((_s_)->frame_list.p[i]) ) \
> + (_s_)->status = GNTST_address_too_big; \
> + else \
> + frame |= 0x80000000U;\
Space before the \. (This is one reason why I hate this style. The
borderline illegibility makes it almost impossible to spot style problems.)
With the adjustment from the previous patch, you'll need a break in here.
But for !valid case, shouldn't we saturate to ~0u ? I recall from the
migration work that various kernels disagree on what constitutes an
invalid MFN.
Then again, I can't see what legitimate case we'd have for a truncation
and an invalid M2P entry needing translating.
~Andrew
> + } \
> + else if ( __copy_to_compat_offset((_d_)->frame_list,
> \
> + i, &frame, 1) ) \
> (_s_)->status = GNTST_bad_virt_addr; \
> } \
> } while (0)
>
>
