gnttab: remove guest_physmap_remove_page() call from gnttab_map_frame()
authorJan Beulich <jbeulich@suse.com>
Fri, 3 Dec 2021 12:54:28 +0000 (13:54 +0100)
committerJan Beulich <jbeulich@suse.com>
Fri, 3 Dec 2021 12:54:28 +0000 (13:54 +0100)
commit1b78d4c63687fbcd0baacb13a7bd97605ffe3b88
treeae7af527886cacb90cc8a18f94a0a644687a907c
parent620ed2c8c777282154a91abca69083a40c9d918d
gnttab: remove guest_physmap_remove_page() call from gnttab_map_frame()

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(); it cannot be dropped there since
xenmem_add_to_physmap_one() doesn't call it either (unlike on x86). 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@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Julien Grall <jgrall@amazon.com>
xen/common/grant_table.c
xen/include/asm-arm/grant_table.h
xen/include/asm-x86/grant_table.h