xen/blkfront: don't use gnttab_query_foreign_access() for mapped status
Commit abf1fd5919d6238ee3bc5eb4a9b6c3947caa6638 upstream. It isn't enough to check whether a grant is still being in use by calling gnttab_query_foreign_access(), as a mapping could be realized by the other side just after having called that function. In case the call was done in preparation of revoking a grant it is better to do so via gnttab_end_foreign_access_ref() and check the success of that operation instead. For the ring allocation use alloc_pages_exact() in order to avoid high order pages in case of a multi-page ring. If a grant wasn't unmapped by the backend without persistent grants being used, set the device state to "error". This is CVE-2022-23036 / part of XSA-396. Reported-by: Demi Marie Obenour <demi@invisiblethingslab.com> Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:

committed by
Greg Kroah-Hartman

parent
3d81e85f30
commit
96219af4e5
@@ -1352,7 +1352,8 @@ free_shadow:
|
|||||||
rinfo->ring_ref[i] = GRANT_INVALID_REF;
|
rinfo->ring_ref[i] = GRANT_INVALID_REF;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
free_pages((unsigned long)rinfo->ring.sring, get_order(info->nr_ring_pages * XEN_PAGE_SIZE));
|
free_pages_exact(rinfo->ring.sring,
|
||||||
|
info->nr_ring_pages * XEN_PAGE_SIZE);
|
||||||
rinfo->ring.sring = NULL;
|
rinfo->ring.sring = NULL;
|
||||||
|
|
||||||
if (rinfo->irq)
|
if (rinfo->irq)
|
||||||
@@ -1436,7 +1437,13 @@ static int blkif_get_final_status(enum blk_req_status s1,
|
|||||||
return BLKIF_RSP_OKAY;
|
return BLKIF_RSP_OKAY;
|
||||||
}
|
}
|
||||||
|
|
||||||
static bool blkif_completion(unsigned long *id,
|
/*
|
||||||
|
* Return values:
|
||||||
|
* 1 response processed.
|
||||||
|
* 0 missing further responses.
|
||||||
|
* -1 error while processing.
|
||||||
|
*/
|
||||||
|
static int blkif_completion(unsigned long *id,
|
||||||
struct blkfront_ring_info *rinfo,
|
struct blkfront_ring_info *rinfo,
|
||||||
struct blkif_response *bret)
|
struct blkif_response *bret)
|
||||||
{
|
{
|
||||||
@@ -1461,7 +1468,7 @@ static bool blkif_completion(unsigned long *id,
|
|||||||
|
|
||||||
/* Wait the second response if not yet here. */
|
/* Wait the second response if not yet here. */
|
||||||
if (s2->status < REQ_DONE)
|
if (s2->status < REQ_DONE)
|
||||||
return false;
|
return 0;
|
||||||
|
|
||||||
bret->status = blkif_get_final_status(s->status,
|
bret->status = blkif_get_final_status(s->status,
|
||||||
s2->status);
|
s2->status);
|
||||||
@@ -1512,42 +1519,43 @@ static bool blkif_completion(unsigned long *id,
|
|||||||
}
|
}
|
||||||
/* Add the persistent grant into the list of free grants */
|
/* Add the persistent grant into the list of free grants */
|
||||||
for (i = 0; i < num_grant; i++) {
|
for (i = 0; i < num_grant; i++) {
|
||||||
if (gnttab_query_foreign_access(s->grants_used[i]->gref)) {
|
if (!gnttab_try_end_foreign_access(s->grants_used[i]->gref)) {
|
||||||
/*
|
/*
|
||||||
* If the grant is still mapped by the backend (the
|
* If the grant is still mapped by the backend (the
|
||||||
* backend has chosen to make this grant persistent)
|
* backend has chosen to make this grant persistent)
|
||||||
* we add it at the head of the list, so it will be
|
* we add it at the head of the list, so it will be
|
||||||
* reused first.
|
* reused first.
|
||||||
*/
|
*/
|
||||||
if (!info->feature_persistent)
|
if (!info->feature_persistent) {
|
||||||
pr_alert_ratelimited("backed has not unmapped grant: %u\n",
|
pr_alert("backed has not unmapped grant: %u\n",
|
||||||
s->grants_used[i]->gref);
|
s->grants_used[i]->gref);
|
||||||
|
return -1;
|
||||||
|
}
|
||||||
list_add(&s->grants_used[i]->node, &rinfo->grants);
|
list_add(&s->grants_used[i]->node, &rinfo->grants);
|
||||||
rinfo->persistent_gnts_c++;
|
rinfo->persistent_gnts_c++;
|
||||||
} else {
|
} else {
|
||||||
/*
|
/*
|
||||||
* If the grant is not mapped by the backend we end the
|
* If the grant is not mapped by the backend we add it
|
||||||
* foreign access and add it to the tail of the list,
|
* to the tail of the list, so it will not be picked
|
||||||
* so it will not be picked again unless we run out of
|
* again unless we run out of persistent grants.
|
||||||
* persistent grants.
|
|
||||||
*/
|
*/
|
||||||
gnttab_end_foreign_access(s->grants_used[i]->gref, 0, 0UL);
|
|
||||||
s->grants_used[i]->gref = GRANT_INVALID_REF;
|
s->grants_used[i]->gref = GRANT_INVALID_REF;
|
||||||
list_add_tail(&s->grants_used[i]->node, &rinfo->grants);
|
list_add_tail(&s->grants_used[i]->node, &rinfo->grants);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (s->req.operation == BLKIF_OP_INDIRECT) {
|
if (s->req.operation == BLKIF_OP_INDIRECT) {
|
||||||
for (i = 0; i < INDIRECT_GREFS(num_grant); i++) {
|
for (i = 0; i < INDIRECT_GREFS(num_grant); i++) {
|
||||||
if (gnttab_query_foreign_access(s->indirect_grants[i]->gref)) {
|
if (!gnttab_try_end_foreign_access(s->indirect_grants[i]->gref)) {
|
||||||
if (!info->feature_persistent)
|
if (!info->feature_persistent) {
|
||||||
pr_alert_ratelimited("backed has not unmapped grant: %u\n",
|
pr_alert("backed has not unmapped grant: %u\n",
|
||||||
s->indirect_grants[i]->gref);
|
s->indirect_grants[i]->gref);
|
||||||
|
return -1;
|
||||||
|
}
|
||||||
list_add(&s->indirect_grants[i]->node, &rinfo->grants);
|
list_add(&s->indirect_grants[i]->node, &rinfo->grants);
|
||||||
rinfo->persistent_gnts_c++;
|
rinfo->persistent_gnts_c++;
|
||||||
} else {
|
} else {
|
||||||
struct page *indirect_page;
|
struct page *indirect_page;
|
||||||
|
|
||||||
gnttab_end_foreign_access(s->indirect_grants[i]->gref, 0, 0UL);
|
|
||||||
/*
|
/*
|
||||||
* Add the used indirect page back to the list of
|
* Add the used indirect page back to the list of
|
||||||
* available pages for indirect grefs.
|
* available pages for indirect grefs.
|
||||||
@@ -1562,7 +1570,7 @@ static bool blkif_completion(unsigned long *id,
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return true;
|
return 1;
|
||||||
}
|
}
|
||||||
|
|
||||||
static irqreturn_t blkif_interrupt(int irq, void *dev_id)
|
static irqreturn_t blkif_interrupt(int irq, void *dev_id)
|
||||||
@@ -1628,12 +1636,17 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (bret.operation != BLKIF_OP_DISCARD) {
|
if (bret.operation != BLKIF_OP_DISCARD) {
|
||||||
|
int ret;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* We may need to wait for an extra response if the
|
* We may need to wait for an extra response if the
|
||||||
* I/O request is split in 2
|
* I/O request is split in 2
|
||||||
*/
|
*/
|
||||||
if (!blkif_completion(&id, rinfo, &bret))
|
ret = blkif_completion(&id, rinfo, &bret);
|
||||||
|
if (!ret)
|
||||||
continue;
|
continue;
|
||||||
|
if (unlikely(ret < 0))
|
||||||
|
goto err;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (add_id_to_freelist(rinfo, id)) {
|
if (add_id_to_freelist(rinfo, id)) {
|
||||||
@@ -1740,8 +1753,7 @@ static int setup_blkring(struct xenbus_device *dev,
|
|||||||
for (i = 0; i < info->nr_ring_pages; i++)
|
for (i = 0; i < info->nr_ring_pages; i++)
|
||||||
rinfo->ring_ref[i] = GRANT_INVALID_REF;
|
rinfo->ring_ref[i] = GRANT_INVALID_REF;
|
||||||
|
|
||||||
sring = (struct blkif_sring *)__get_free_pages(GFP_NOIO | __GFP_HIGH,
|
sring = alloc_pages_exact(ring_size, GFP_NOIO);
|
||||||
get_order(ring_size));
|
|
||||||
if (!sring) {
|
if (!sring) {
|
||||||
xenbus_dev_fatal(dev, -ENOMEM, "allocating shared ring");
|
xenbus_dev_fatal(dev, -ENOMEM, "allocating shared ring");
|
||||||
return -ENOMEM;
|
return -ENOMEM;
|
||||||
@@ -1751,7 +1763,7 @@ static int setup_blkring(struct xenbus_device *dev,
|
|||||||
|
|
||||||
err = xenbus_grant_ring(dev, rinfo->ring.sring, info->nr_ring_pages, gref);
|
err = xenbus_grant_ring(dev, rinfo->ring.sring, info->nr_ring_pages, gref);
|
||||||
if (err < 0) {
|
if (err < 0) {
|
||||||
free_pages((unsigned long)sring, get_order(ring_size));
|
free_pages_exact(sring, ring_size);
|
||||||
rinfo->ring.sring = NULL;
|
rinfo->ring.sring = NULL;
|
||||||
goto fail;
|
goto fail;
|
||||||
}
|
}
|
||||||
@@ -2729,11 +2741,10 @@ static void purge_persistent_grants(struct blkfront_info *info)
|
|||||||
list_for_each_entry_safe(gnt_list_entry, tmp, &rinfo->grants,
|
list_for_each_entry_safe(gnt_list_entry, tmp, &rinfo->grants,
|
||||||
node) {
|
node) {
|
||||||
if (gnt_list_entry->gref == GRANT_INVALID_REF ||
|
if (gnt_list_entry->gref == GRANT_INVALID_REF ||
|
||||||
gnttab_query_foreign_access(gnt_list_entry->gref))
|
!gnttab_try_end_foreign_access(gnt_list_entry->gref))
|
||||||
continue;
|
continue;
|
||||||
|
|
||||||
list_del(&gnt_list_entry->node);
|
list_del(&gnt_list_entry->node);
|
||||||
gnttab_end_foreign_access(gnt_list_entry->gref, 0, 0UL);
|
|
||||||
rinfo->persistent_gnts_c--;
|
rinfo->persistent_gnts_c--;
|
||||||
gnt_list_entry->gref = GRANT_INVALID_REF;
|
gnt_list_entry->gref = GRANT_INVALID_REF;
|
||||||
list_add_tail(&gnt_list_entry->node, &rinfo->grants);
|
list_add_tail(&gnt_list_entry->node, &rinfo->grants);
|
||||||
|
Reference in New Issue
Block a user