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:
parent
782e5ebcc8
commit
0e35f3ab69
@ -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;
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user