On 02.12.2021 10:09, Julien Grall wrote: > 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 <[email protected]> > > 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 <[email protected]>
Thanks, but let me ask for a clarification: Explicitly just the Arm part, or also the common code change? I ask because from the x86 side I already have an ack by Roger, but strictly speaking that doesn't cover common code. Jan
