Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f3065750 authored by Juergen Gross's avatar Juergen Gross Committed by Greg Kroah-Hartman
Browse files

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: 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>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 73e1d9b3
Loading
Loading
Loading
Loading
+39 −28
Original line number Diff line number Diff line
@@ -1266,17 +1266,16 @@ static void blkif_free_ring(struct blkfront_ring_info *rinfo)
		list_for_each_entry_safe(persistent_gnt, n,
					 &rinfo->grants, node) {
			list_del(&persistent_gnt->node);
			if (persistent_gnt->gref != GRANT_INVALID_REF) {
				gnttab_end_foreign_access(persistent_gnt->gref,
							  0, 0UL);
			if (persistent_gnt->gref == GRANT_INVALID_REF ||
			    !gnttab_try_end_foreign_access(persistent_gnt->gref))
				continue;

			rinfo->persistent_gnts_c--;
			}
			if (info->feature_persistent)
				__free_page(persistent_gnt->page);
			kfree(persistent_gnt);
		}
	}
	BUG_ON(rinfo->persistent_gnts_c != 0);

	for (i = 0; i < BLK_RING_SIZE(info); i++) {
		/*
@@ -1333,7 +1332,8 @@ static void blkif_free_ring(struct blkfront_ring_info *rinfo)
			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;

	if (rinfo->irq)
@@ -1417,7 +1417,13 @@ static int blkif_get_final_status(enum blk_req_status s1,
	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 blkif_response *bret)
{
@@ -1493,42 +1499,43 @@ static bool blkif_completion(unsigned long *id,
	}
	/* Add the persistent grant into the list of free grants */
	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
			 * backend has chosen to make this grant persistent)
			 * we add it at the head of the list, so it will be
			 * reused first.
			 */
			if (!info->feature_persistent)
				pr_alert_ratelimited("backed has not unmapped grant: %u\n",
			if (!info->feature_persistent) {
				pr_alert("backed has not unmapped grant: %u\n",
					 s->grants_used[i]->gref);
				return -1;
			}
			list_add(&s->grants_used[i]->node, &rinfo->grants);
			rinfo->persistent_gnts_c++;
		} else {
			/*
			 * If the grant is not mapped by the backend we end the
			 * foreign access and add it to the tail of the list,
			 * so it will not be picked again unless we run out of
			 * persistent grants.
			 * If the grant is not mapped by the backend we add it
			 * to the tail of the list, so it will not be picked
			 * again unless we run out of persistent grants.
			 */
			gnttab_end_foreign_access(s->grants_used[i]->gref, 0, 0UL);
			s->grants_used[i]->gref = GRANT_INVALID_REF;
			list_add_tail(&s->grants_used[i]->node, &rinfo->grants);
		}
	}
	if (s->req.operation == BLKIF_OP_INDIRECT) {
		for (i = 0; i < INDIRECT_GREFS(num_grant); i++) {
			if (gnttab_query_foreign_access(s->indirect_grants[i]->gref)) {
				if (!info->feature_persistent)
					pr_alert_ratelimited("backed has not unmapped grant: %u\n",
			if (!gnttab_try_end_foreign_access(s->indirect_grants[i]->gref)) {
				if (!info->feature_persistent) {
					pr_alert("backed has not unmapped grant: %u\n",
						 s->indirect_grants[i]->gref);
					return -1;
				}
				list_add(&s->indirect_grants[i]->node, &rinfo->grants);
				rinfo->persistent_gnts_c++;
			} else {
				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
				 * available pages for indirect grefs.
@@ -1610,12 +1617,17 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
		}

		if (bret.operation != BLKIF_OP_DISCARD) {
			int ret;

			/*
			 * We may need to wait for an extra response if the
			 * I/O request is split in 2
			 */
			if (!blkif_completion(&id, rinfo, &bret))
			ret = blkif_completion(&id, rinfo, &bret);
			if (!ret)
				continue;
			if (unlikely(ret < 0))
				goto err;
		}

		if (add_id_to_freelist(rinfo, id)) {
@@ -1717,8 +1729,7 @@ static int setup_blkring(struct xenbus_device *dev,
	for (i = 0; i < info->nr_ring_pages; i++)
		rinfo->ring_ref[i] = GRANT_INVALID_REF;

	sring = (struct blkif_sring *)__get_free_pages(GFP_NOIO | __GFP_HIGH,
						       get_order(ring_size));
	sring = alloc_pages_exact(ring_size, GFP_NOIO);
	if (!sring) {
		xenbus_dev_fatal(dev, -ENOMEM, "allocating shared ring");
		return -ENOMEM;
@@ -1728,7 +1739,7 @@ static int setup_blkring(struct xenbus_device *dev,

	err = xenbus_grant_ring(dev, rinfo->ring.sring, info->nr_ring_pages, gref);
	if (err < 0) {
		free_pages((unsigned long)sring, get_order(ring_size));
		free_pages_exact(sring, ring_size);
		rinfo->ring.sring = NULL;
		goto fail;
	}