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

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

xen/netfront: react properly to failing gnttab_end_foreign_access_ref() 



Commit 66e3531b33ee51dad17c463b4d9c9f52e341503d upstream.

When calling gnttab_end_foreign_access_ref() the returned value must
be tested and the reaction to that value should be appropriate.

In case of failure in xennet_get_responses() the reaction should not be
to crash the system, but to disable the network device.

The calls in setup_netfront() can be replaced by calls of
gnttab_end_foreign_access(). While at it avoid double free of ring
pages and grant references via xennet_disconnect_backend() in this case.

This is CVE-2022-23042 / 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 avatarJan Beulich <jbeulich@suse.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 92dc0e4a

drivers/net/xen-netfront.c

+31 −17
Original line number Original line Diff line number Diff line
@@ -862,7 +862,6 @@ static int xennet_get_responses(struct netfront_queue *queue, 
	int max = XEN_NETIF_NR_SLOTS_MIN + (rx->status <= RX_COPY_THRESHOLD);

	int max = XEN_NETIF_NR_SLOTS_MIN + (rx->status <= RX_COPY_THRESHOLD);

	int slots = 1;

	int slots = 1;

	int err = 0;

	int err = 0;

	unsigned long ret;





	if (rx->flags & XEN_NETRXF_extra_info) {

	if (rx->flags & XEN_NETRXF_extra_info) {

		err = xennet_get_extras(queue, extras, rp);

		err = xennet_get_extras(queue, extras, rp);
@@ -893,8 +892,13 @@ static int xennet_get_responses(struct netfront_queue *queue, 
			goto next;

			goto next;

		}

		}





		ret = gnttab_end_foreign_access_ref(ref, 0);

		if (!gnttab_end_foreign_access_ref(ref, 0)) {

		BUG_ON(!ret);

			dev_alert(dev,

				  "Grant still in use by backend domain\n");

			queue->info->broken = true;

			dev_alert(dev, "Disabled for further use\n");

			return -EINVAL;

		}





		gnttab_release_grant_reference(&queue->gref_rx_head, ref);

		gnttab_release_grant_reference(&queue->gref_rx_head, ref);
@@ -1098,6 +1102,10 @@ static int xennet_poll(struct napi_struct *napi, int budget) 
		err = xennet_get_responses(queue, &rinfo, rp, &tmpq);

		err = xennet_get_responses(queue, &rinfo, rp, &tmpq);





		if (unlikely(err)) {

		if (unlikely(err)) {

			if (queue->info->broken) {

				spin_unlock(&queue->rx_lock);

				return 0;

			}

err:

err:

			while ((skb = __skb_dequeue(&tmpq)))

			while ((skb = __skb_dequeue(&tmpq)))

				__skb_queue_tail(&errq, skb);

				__skb_queue_tail(&errq, skb);
@@ -1676,7 +1684,7 @@ static int setup_netfront(struct xenbus_device *dev, 
			struct netfront_queue *queue, unsigned int feature_split_evtchn)

			struct netfront_queue *queue, unsigned int feature_split_evtchn)

{

{

	struct xen_netif_tx_sring *txs;

	struct xen_netif_tx_sring *txs;

	struct xen_netif_rx_sring *rxs;

	struct xen_netif_rx_sring *rxs = NULL;

	grant_ref_t gref;

	grant_ref_t gref;

	int err;

	int err;
@@ -1696,21 +1704,21 @@ static int setup_netfront(struct xenbus_device *dev, 




	err = xenbus_grant_ring(dev, txs, 1, &gref);

	err = xenbus_grant_ring(dev, txs, 1, &gref);

	if (err < 0)

	if (err < 0)

		goto grant_tx_ring_fail;

		goto fail;

	queue->tx_ring_ref = gref;

	queue->tx_ring_ref = gref;





	rxs = (struct xen_netif_rx_sring *)get_zeroed_page(GFP_NOIO | __GFP_HIGH);

	rxs = (struct xen_netif_rx_sring *)get_zeroed_page(GFP_NOIO | __GFP_HIGH);

	if (!rxs) {

	if (!rxs) {

		err = -ENOMEM;

		err = -ENOMEM;

		xenbus_dev_fatal(dev, err, "allocating rx ring page");

		xenbus_dev_fatal(dev, err, "allocating rx ring page");

		goto alloc_rx_ring_fail;

		goto fail;

	}

	}

	SHARED_RING_INIT(rxs);

	SHARED_RING_INIT(rxs);

	FRONT_RING_INIT(&queue->rx, rxs, XEN_PAGE_SIZE);

	FRONT_RING_INIT(&queue->rx, rxs, XEN_PAGE_SIZE);





	err = xenbus_grant_ring(dev, rxs, 1, &gref);

	err = xenbus_grant_ring(dev, rxs, 1, &gref);

	if (err < 0)

	if (err < 0)

		goto grant_rx_ring_fail;

		goto fail;

	queue->rx_ring_ref = gref;

	queue->rx_ring_ref = gref;





	if (feature_split_evtchn)

	if (feature_split_evtchn)
@@ -1723,22 +1731,28 @@ static int setup_netfront(struct xenbus_device *dev, 
		err = setup_netfront_single(queue);

		err = setup_netfront_single(queue);





	if (err)

	if (err)

		goto alloc_evtchn_fail;

		goto fail;





	return 0;

	return 0;





	/* If we fail to setup netfront, it is safe to just revoke access to

	/* If we fail to setup netfront, it is safe to just revoke access to

	 * granted pages because backend is not accessing it at this point.

	 * granted pages because backend is not accessing it at this point.

	 */

	 */

alloc_evtchn_fail:

 fail:

	gnttab_end_foreign_access_ref(queue->rx_ring_ref, 0);

	if (queue->rx_ring_ref != GRANT_INVALID_REF) {

grant_rx_ring_fail:

		gnttab_end_foreign_access(queue->rx_ring_ref, 0,

					  (unsigned long)rxs);

		queue->rx_ring_ref = GRANT_INVALID_REF;

	} else {

		free_page((unsigned long)rxs);

		free_page((unsigned long)rxs);

alloc_rx_ring_fail:

	}

	gnttab_end_foreign_access_ref(queue->tx_ring_ref, 0);

	if (queue->tx_ring_ref != GRANT_INVALID_REF) {

grant_tx_ring_fail:

		gnttab_end_foreign_access(queue->tx_ring_ref, 0,

					  (unsigned long)txs);

		queue->tx_ring_ref = GRANT_INVALID_REF;

	} else {

		free_page((unsigned long)txs);

		free_page((unsigned long)txs);

fail:

	}

	return err;

	return err;

}

}