Re: [Xen-devel] XSA-255 and Arm

[Julien Grall <julien@xxxxxxx>]

Hi,

On 10/12/2019 10:42, George Dunlap wrote:
> On 12/9/19 1:13 PM, Julien Grall wrote:
> > Hi all,
> >
> > I was looking at the Grant Table code over the week-end and noticed
> > thart XSA-255 [1] introduced some unintended consequences on Arm.
> >
> > Since the XSA, gnttab_map_frame() will remove the previous mapping (if
> > any) because mapping to the new GFN.
> >
> > As on Arm we don't have an M2P, the GFN is stored per frame in the
> > grant-table code. This will never get cleared during unmapping (e.g.
> > XENMEM_remove_from_physmap) and therefore we may end up to remove a
> > mapping from someone different (Arm does not check the MFN is the
> > correct one before removing mapping).
> Sorry Julien,  I don't know enough about the ARM grant mapping code to
> know what this is about.  Can you point me to the relevant files /
> functions / structures, and maybe sketch out a problematic behavior?
If you look at the implementation of gnttab_map_frame() 
(common/grant_table.c), it will use gnttab_get_frame_gfn(...) to know 
whether the grant-table frame was already mapped in the Guest P2M.
If it is already mapped, then we will try to remove the current mapping 
before doing the new one.
On Arm, gnttab_get_frame_gfn() are using an internal array because to we 
don't have an internal array because we don't have an M2P. There is a 
sister function to set the frame (see gnttab_set_frame_gfn()). However, 
this is only called when mapping the frame and not when the unmap is 
done XENMEM_remove_from_physmap.
From my understanding, the current code (i.e remove the old mapping) is 
trying to workaround the fact we don't take reference when doing a 
mapping in the page-tables.
At the default of not handling reference for all the mappings, we could 
handle grant frame in a similar way to foreign mapping. This would 
remove the restriction on the number of mappings and we could update the 
internal array at the same time.
The only concern with this appraoch is we would have to walk through the 
array to find the GFN.
Cheers,

--
Julien Grall

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