lkml.org 
[lkml]   [2022]   [Mar]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.10 57/58] xen/netfront: react properly to failing gnttab_end_foreign_access_ref()
    Date
    From: Juergen Gross <jgross@suse.com>

    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>
    ---
    drivers/net/xen-netfront.c | 48 +++++++++++++++++++++++++++++----------------
    1 file changed, 31 insertions(+), 17 deletions(-)

    --- a/drivers/net/xen-netfront.c
    +++ b/drivers/net/xen-netfront.c
    @@ -990,7 +990,6 @@ static int xennet_get_responses(struct n
    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;
    @@ -1032,8 +1031,13 @@ static int xennet_get_responses(struct n
    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);

    @@ -1254,6 +1258,10 @@ static int xennet_poll(struct napi_struc
    &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);
    @@ -1918,7 +1926,7 @@ static int setup_netfront(struct xenbus_
    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;

    @@ -1938,21 +1946,21 @@ static int setup_netfront(struct xenbus_

    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)
    @@ -1965,22 +1973,28 @@ static int setup_netfront(struct xenbus_
    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;
    }


    \
     
     \ /
      Last update: 2022-03-10 15:44    [W:3.332 / U:0.692 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site