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: Demi Marie Obenour <demi@invisiblethingslab.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
Juergen Gross 2022-02-25 16:05:43 +01:00 committed by Greg Kroah-Hartman
parent 782e5ebcc8
commit 0e35f3ab69

View File

@ -859,7 +859,6 @@ static int xennet_get_responses(struct netfront_queue *queue,
int max = XEN_NETIF_NR_SLOTS_MIN + (rx->status <= RX_COPY_THRESHOLD);
int slots = 1;
int err = 0;
unsigned long ret;
if (rx->flags & XEN_NETRXF_extra_info) {
err = xennet_get_extras(queue, extras, rp);
@ -890,8 +889,13 @@ static int xennet_get_responses(struct netfront_queue *queue,
goto next;
}
ret = gnttab_end_foreign_access_ref(ref, 0);
BUG_ON(!ret);
if (!gnttab_end_foreign_access_ref(ref, 0)) {
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);
@ -1095,6 +1099,10 @@ static int xennet_poll(struct napi_struct *napi, int budget)
err = xennet_get_responses(queue, &rinfo, rp, &tmpq);
if (unlikely(err)) {
if (queue->info->broken) {
spin_unlock(&queue->rx_lock);
return 0;
}
err:
while ((skb = __skb_dequeue(&tmpq)))
__skb_queue_tail(&errq, skb);
@ -1673,7 +1681,7 @@ static int setup_netfront(struct xenbus_device *dev,
struct netfront_queue *queue, unsigned int feature_split_evtchn)
{
struct xen_netif_tx_sring *txs;
struct xen_netif_rx_sring *rxs;
struct xen_netif_rx_sring *rxs = NULL;
grant_ref_t gref;
int err;
@ -1693,21 +1701,21 @@ static int setup_netfront(struct xenbus_device *dev,
err = xenbus_grant_ring(dev, txs, 1, &gref);
if (err < 0)
goto grant_tx_ring_fail;
goto fail;
queue->tx_ring_ref = gref;
rxs = (struct xen_netif_rx_sring *)get_zeroed_page(GFP_NOIO | __GFP_HIGH);
if (!rxs) {
err = -ENOMEM;
xenbus_dev_fatal(dev, err, "allocating rx ring page");
goto alloc_rx_ring_fail;
goto fail;
}
SHARED_RING_INIT(rxs);
FRONT_RING_INIT(&queue->rx, rxs, XEN_PAGE_SIZE);
err = xenbus_grant_ring(dev, rxs, 1, &gref);
if (err < 0)
goto grant_rx_ring_fail;
goto fail;
queue->rx_ring_ref = gref;
if (feature_split_evtchn)
@ -1720,22 +1728,28 @@ static int setup_netfront(struct xenbus_device *dev,
err = setup_netfront_single(queue);
if (err)
goto alloc_evtchn_fail;
goto fail;
return 0;
/* 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.
*/
alloc_evtchn_fail:
gnttab_end_foreign_access_ref(queue->rx_ring_ref, 0);
grant_rx_ring_fail:
free_page((unsigned long)rxs);
alloc_rx_ring_fail:
gnttab_end_foreign_access_ref(queue->tx_ring_ref, 0);
grant_tx_ring_fail:
free_page((unsigned long)txs);
fail:
fail:
if (queue->rx_ring_ref != GRANT_INVALID_REF) {
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);
}
if (queue->tx_ring_ref != GRANT_INVALID_REF) {
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);
}
return err;
}