xen/netfront: react properly to failing gnttab_end_foreign_access_ref()

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>
---
V2:
- avoid double free
V3:
- remove pointless initializer (Jan Beulich)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 13926d03418ef6061cc4b8a6e08ff48babef7aa0..daa4e6106aacc4004c4b02b2daf46fe969ee8d66 100644 (file)
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -988,7 +988,6 @@ static int xennet_get_responses(struct netfront_queue *queue,
        struct device *dev = &queue->info->netdev->dev;
        struct bpf_prog *xdp_prog;
        struct xdp_buff xdp;
-       unsigned long ret;
        int slots = 1;
        int err = 0;
        u32 verdict;
@@ -1030,8 +1029,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);
 
@@ -1252,6 +1256,10 @@ static int xennet_poll(struct napi_struct *napi, int budget)
                                           &need_xdp_flush);
 
                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);
@@ -1916,7 +1924,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;
 
@@ -1936,21 +1944,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)
@@ -1963,22 +1971,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;
 }