Commit abf1fd59 authored by Juergen Gross's avatar Juergen Gross

xen/blkfront: don't use gnttab_query_foreign_access() for mapped status

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: default avatarDemi Marie Obenour <demi@invisiblethingslab.com>
Signed-off-by: default avatarJuergen Gross <jgross@suse.com>
Reviewed-by: default avatarRoger Pau Monné <roger.pau@citrix.com>
---
V2:
- use gnttab_try_end_foreign_access()
V4:
- use alloc_pages_exact() and free_pages_exact()
- set state to error if backend didn't unmap (Roger Pau Monné)
parent 6b1775f2
...@@ -1288,7 +1288,8 @@ static void blkif_free_ring(struct blkfront_ring_info *rinfo) ...@@ -1288,7 +1288,8 @@ static void blkif_free_ring(struct blkfront_ring_info *rinfo)
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)
...@@ -1372,9 +1373,15 @@ static int blkif_get_final_status(enum blk_req_status s1, ...@@ -1372,9 +1373,15 @@ 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, /*
struct blkfront_ring_info *rinfo, * Return values:
struct blkif_response *bret) * 1 response processed.
* 0 missing further responses.
* -1 error while processing.
*/
static int blkif_completion(unsigned long *id,
struct blkfront_ring_info *rinfo,
struct blkif_response *bret)
{ {
int i = 0; int i = 0;
struct scatterlist *sg; struct scatterlist *sg;
...@@ -1397,7 +1404,7 @@ static bool blkif_completion(unsigned long *id, ...@@ -1397,7 +1404,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);
...@@ -1448,42 +1455,43 @@ static bool blkif_completion(unsigned long *id, ...@@ -1448,42 +1455,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.
...@@ -1498,7 +1506,7 @@ static bool blkif_completion(unsigned long *id, ...@@ -1498,7 +1506,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)
...@@ -1564,12 +1572,17 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id) ...@@ -1564,12 +1572,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)) {
...@@ -1676,8 +1689,7 @@ static int setup_blkring(struct xenbus_device *dev, ...@@ -1676,8 +1689,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;
...@@ -1687,7 +1699,7 @@ static int setup_blkring(struct xenbus_device *dev, ...@@ -1687,7 +1699,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;
} }
...@@ -2532,11 +2544,10 @@ static void purge_persistent_grants(struct blkfront_info *info) ...@@ -2532,11 +2544,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);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment