Skip to content

Commit

Permalink
xen/xenbus: don't let xenbus_grant_ring() remove grants in error case
Browse files Browse the repository at this point in the history
Commit 3777ea7bac3113005b7180e6b9dadf16d19a5827 upstream.

Letting xenbus_grant_ring() tear down grants in the error case is
problematic, as the other side could already have used these grants.
Calling gnttab_end_foreign_access_ref() without checking success is
resulting in an unclear situation for any caller of xenbus_grant_ring()
as in the error case the memory pages of the ring page might be
partially mapped. Freeing them would risk unwanted foreign access to
them, while not freeing them would leak memory.

In order to remove the need to undo any gnttab_grant_foreign_access()
calls, use gnttab_alloc_grant_references() to make sure no further
error can occur in the loop granting access to the ring pages.

It should be noted that this way of handling removes leaking of
grant entries in the error case, too.

This is CVE-2022-23040 / part of XSA-396.

Reported-by: Demi Marie Obenour <[email protected]>
Signed-off-by: Juergen Gross <[email protected]>
Reviewed-by: Jan Beulich <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
  • Loading branch information
jgross1 authored and gregkh committed Mar 11, 2022
1 parent 623ca87 commit 8d521d9
Showing 1 changed file with 11 additions and 13 deletions.
24 changes: 11 additions & 13 deletions drivers/xen/xenbus/xenbus_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,14 @@ int xenbus_grant_ring(struct xenbus_device *dev, void *vaddr,
unsigned int nr_pages, grant_ref_t *grefs)
{
int err;
int i, j;
unsigned int i;
grant_ref_t gref_head;

err = gnttab_alloc_grant_references(nr_pages, &gref_head);
if (err) {
xenbus_dev_fatal(dev, err, "granting access to ring page");
return err;
}

for (i = 0; i < nr_pages; i++) {
unsigned long gfn;
Expand All @@ -378,23 +385,14 @@ int xenbus_grant_ring(struct xenbus_device *dev, void *vaddr,
else
gfn = virt_to_gfn(vaddr);

err = gnttab_grant_foreign_access(dev->otherend_id, gfn, 0);
if (err < 0) {
xenbus_dev_fatal(dev, err,
"granting access to ring page");
goto fail;
}
grefs[i] = err;
grefs[i] = gnttab_claim_grant_reference(&gref_head);
gnttab_grant_foreign_access_ref(grefs[i], dev->otherend_id,
gfn, 0);

vaddr = vaddr + XEN_PAGE_SIZE;
}

return 0;

fail:
for (j = 0; j < i; j++)
gnttab_end_foreign_access_ref(grefs[j], 0);
return err;
}
EXPORT_SYMBOL_GPL(xenbus_grant_ring);

Expand Down

0 comments on commit 8d521d9

Please sign in to comment.