Skip to content

Commit

Permalink
Xen/gnttab: handle p2m update errors on a per-slot basis
Browse files Browse the repository at this point in the history
Bailing immediately from set_foreign_p2m_mapping() upon a p2m updating
error leaves the full batch in an ambiguous state as far as the caller
is concerned. Instead flags respective slots as bad, unmapping what
was mapped there right away.

HYPERVISOR_grant_table_op()'s return value and the individual unmap
slots' status fields get used only for a one-time - there's not much we
can do in case of a failure.

Note that there's no GNTST_enomem or alike, so GNTST_general_error gets
used.

The map ops' handle fields get overwritten just to be on the safe side.

This is part of XSA-367.

Cc: <[email protected]>
Signed-off-by: Jan Beulich <[email protected]>
Reviewed-by: Juergen Gross <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Juergen Gross <[email protected]>
  • Loading branch information
jbeulich authored and jgross1 committed Mar 3, 2021
1 parent 53f131c commit 8310b77
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 7 deletions.
35 changes: 31 additions & 4 deletions arch/arm/xen/p2m.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,39 @@ int set_foreign_p2m_mapping(struct gnttab_map_grant_ref *map_ops,
int i;

for (i = 0; i < count; i++) {
struct gnttab_unmap_grant_ref unmap;
int rc;

if (map_ops[i].status)
continue;
if (unlikely(!set_phys_to_machine(map_ops[i].host_addr >> XEN_PAGE_SHIFT,
map_ops[i].dev_bus_addr >> XEN_PAGE_SHIFT))) {
return -ENOMEM;
}
if (likely(set_phys_to_machine(map_ops[i].host_addr >> XEN_PAGE_SHIFT,
map_ops[i].dev_bus_addr >> XEN_PAGE_SHIFT)))
continue;

/*
* Signal an error for this slot. This in turn requires
* immediate unmapping.
*/
map_ops[i].status = GNTST_general_error;
unmap.host_addr = map_ops[i].host_addr,
unmap.handle = map_ops[i].handle;
map_ops[i].handle = ~0;
if (map_ops[i].flags & GNTMAP_device_map)
unmap.dev_bus_addr = map_ops[i].dev_bus_addr;
else
unmap.dev_bus_addr = 0;

/*
* Pre-populate the status field, to be recognizable in
* the log message below.
*/
unmap.status = 1;

rc = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref,
&unmap, 1);
if (rc || unmap.status != GNTST_okay)
pr_err_once("gnttab unmap failed: rc=%d st=%d\n",
rc, unmap.status);
}

return 0;
Expand Down
44 changes: 41 additions & 3 deletions arch/x86/xen/p2m.c
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,8 @@ int set_foreign_p2m_mapping(struct gnttab_map_grant_ref *map_ops,

for (i = 0; i < count; i++) {
unsigned long mfn, pfn;
struct gnttab_unmap_grant_ref unmap[2];
int rc;

/* Do not add to override if the map failed. */
if (map_ops[i].status != GNTST_okay ||
Expand All @@ -727,10 +729,46 @@ int set_foreign_p2m_mapping(struct gnttab_map_grant_ref *map_ops,

WARN(pfn_to_mfn(pfn) != INVALID_P2M_ENTRY, "page must be ballooned");

if (unlikely(!set_phys_to_machine(pfn, FOREIGN_FRAME(mfn)))) {
ret = -ENOMEM;
goto out;
if (likely(set_phys_to_machine(pfn, FOREIGN_FRAME(mfn))))
continue;

/*
* Signal an error for this slot. This in turn requires
* immediate unmapping.
*/
map_ops[i].status = GNTST_general_error;
unmap[0].host_addr = map_ops[i].host_addr,
unmap[0].handle = map_ops[i].handle;
map_ops[i].handle = ~0;
if (map_ops[i].flags & GNTMAP_device_map)
unmap[0].dev_bus_addr = map_ops[i].dev_bus_addr;
else
unmap[0].dev_bus_addr = 0;

if (kmap_ops) {
kmap_ops[i].status = GNTST_general_error;
unmap[1].host_addr = kmap_ops[i].host_addr,
unmap[1].handle = kmap_ops[i].handle;
kmap_ops[i].handle = ~0;
if (kmap_ops[i].flags & GNTMAP_device_map)
unmap[1].dev_bus_addr = kmap_ops[i].dev_bus_addr;
else
unmap[1].dev_bus_addr = 0;
}

/*
* Pre-populate both status fields, to be recognizable in
* the log message below.
*/
unmap[0].status = 1;
unmap[1].status = 1;

rc = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref,
unmap, 1 + !!kmap_ops);
if (rc || unmap[0].status != GNTST_okay ||
unmap[1].status != GNTST_okay)
pr_err_once("gnttab unmap failed: rc=%d st0=%d st1=%d\n",
rc, unmap[0].status, unmap[1].status);
}

out:
Expand Down

0 comments on commit 8310b77

Please sign in to comment.