Re: [PATCH 1/2] gnttab: remove guest_physmap_remove_page() call from gnttab_map_frame()

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 13/09/2021 07:41, Jan Beulich wrote:
> Without holding appropriate locks, attempting to remove a prior mapping
> of the underlying page is pointless, as the same (or another) mapping
> could be re-established by a parallel request on another vCPU. Move the
> code to Arm's gnttab_set_frame_gfn(). Of course this new placement
> doesn't improve things in any way as far as the security of grant status
> frame mappings goes (see XSA-379). Proper locking would be needed here
> to allow status frames to be mapped securely.
>
> In turn this then requires replacing the other use in
> gnttab_unpopulate_status_frames(), which yet in turn requires adjusting
> x86's gnttab_set_frame_gfn(). Note that with proper locking inside
> guest_physmap_remove_page() combined with checking the GFN's mapping
> there against the passed in MFN, there then is no issue with the
> involved multiple gnttab_set_frame_gfn()-s potentially returning varying
> values (due to a racing XENMAPSPACE_grant_table request).
>
> This, as a side effect, does away with gnttab_map_frame() having a local
> variable "gfn" which shadows a function parameter of the same name.
>
> Together with XSA-379 this points out that XSA-255's addition to
> gnttab_map_frame() was really useless.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
As discussed in the thread, I expect the Arm part to be simplified after 
Oleksandr's series. So for the Arm part:
Acked-by: Julien Grall <jgrall@xxxxxxxxxx>

Cheers,

--
Julien Grall