All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Juergen Gross <jgross@suse.com>,
	Jan Beulich <jbeulich@suse.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: [PATCH 4.19 68/69] xen/netfront: dont trust the backend response data blindly
Date: Mon, 29 Nov 2021 19:18:50 +0100	[thread overview]
Message-ID: <20211129181705.862563937@linuxfoundation.org> (raw)
In-Reply-To: <20211129181703.670197996@linuxfoundation.org>

From: Juergen Gross <jgross@suse.com>

commit a884daa61a7d91650987e855464526aef219590f upstream.

Today netfront will trust the backend to send only sane response data.
In order to avoid privilege escalations or crashes in case of malicious
backends verify the data to be within expected limits. Especially make
sure that the response always references an outstanding request.

Note that only the tx queue needs special id handling, as for the rx
queue the id is equal to the index in the ring page.

Introduce a new indicator for the device whether it is broken and let
the device stop working when it is set. Set this indicator in case the
backend sets any weird data.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/net/xen-netfront.c |   80 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 75 insertions(+), 5 deletions(-)

--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -126,10 +126,12 @@ struct netfront_queue {
 	struct sk_buff *tx_skbs[NET_TX_RING_SIZE];
 	unsigned short tx_link[NET_TX_RING_SIZE];
 #define TX_LINK_NONE 0xffff
+#define TX_PENDING   0xfffe
 	grant_ref_t gref_tx_head;
 	grant_ref_t grant_tx_ref[NET_TX_RING_SIZE];
 	struct page *grant_tx_page[NET_TX_RING_SIZE];
 	unsigned tx_skb_freelist;
+	unsigned int tx_pend_queue;
 
 	spinlock_t   rx_lock ____cacheline_aligned_in_smp;
 	struct xen_netif_rx_front_ring rx;
@@ -155,6 +157,9 @@ struct netfront_info {
 	struct netfront_stats __percpu *rx_stats;
 	struct netfront_stats __percpu *tx_stats;
 
+	/* Is device behaving sane? */
+	bool broken;
+
 	atomic_t rx_gso_checksum_fixup;
 };
 
@@ -339,7 +344,7 @@ static int xennet_open(struct net_device
 	unsigned int i = 0;
 	struct netfront_queue *queue = NULL;
 
-	if (!np->queues)
+	if (!np->queues || np->broken)
 		return -ENODEV;
 
 	for (i = 0; i < num_queues; ++i) {
@@ -367,11 +372,17 @@ static void xennet_tx_buf_gc(struct netf
 	unsigned short id;
 	struct sk_buff *skb;
 	bool more_to_do;
+	const struct device *dev = &queue->info->netdev->dev;
 
 	BUG_ON(!netif_carrier_ok(queue->info->netdev));
 
 	do {
 		prod = queue->tx.sring->rsp_prod;
+		if (RING_RESPONSE_PROD_OVERFLOW(&queue->tx, prod)) {
+			dev_alert(dev, "Illegal number of responses %u\n",
+				  prod - queue->tx.rsp_cons);
+			goto err;
+		}
 		rmb(); /* Ensure we see responses up to 'rp'. */
 
 		for (cons = queue->tx.rsp_cons; cons != prod; cons++) {
@@ -381,14 +392,27 @@ static void xennet_tx_buf_gc(struct netf
 			if (txrsp.status == XEN_NETIF_RSP_NULL)
 				continue;
 
-			id  = txrsp.id;
+			id = txrsp.id;
+			if (id >= RING_SIZE(&queue->tx)) {
+				dev_alert(dev,
+					  "Response has incorrect id (%u)\n",
+					  id);
+				goto err;
+			}
+			if (queue->tx_link[id] != TX_PENDING) {
+				dev_alert(dev,
+					  "Response for inactive request\n");
+				goto err;
+			}
+
+			queue->tx_link[id] = TX_LINK_NONE;
 			skb = queue->tx_skbs[id];
 			queue->tx_skbs[id] = NULL;
 			if (unlikely(gnttab_query_foreign_access(
 				queue->grant_tx_ref[id]) != 0)) {
-				pr_alert("%s: warning -- grant still in use by backend domain\n",
-					 __func__);
-				BUG();
+				dev_alert(dev,
+					  "Grant still in use by backend domain\n");
+				goto err;
 			}
 			gnttab_end_foreign_access_ref(
 				queue->grant_tx_ref[id], GNTMAP_readonly);
@@ -406,6 +430,12 @@ static void xennet_tx_buf_gc(struct netf
 	} while (more_to_do);
 
 	xennet_maybe_wake_tx(queue);
+
+	return;
+
+ err:
+	queue->info->broken = true;
+	dev_alert(dev, "Disabled for further use\n");
 }
 
 struct xennet_gnttab_make_txreq {
@@ -449,6 +479,12 @@ static void xennet_tx_setup_grant(unsign
 
 	*tx = info->tx_local;
 
+	/*
+	 * Put the request in the pending queue, it will be set to be pending
+	 * when the producer index is about to be raised.
+	 */
+	add_id_to_list(&queue->tx_pend_queue, queue->tx_link, id);
+
 	info->tx = tx;
 	info->size += info->tx_local.size;
 }
@@ -542,6 +578,15 @@ static u16 xennet_select_queue(struct ne
 	return queue_idx;
 }
 
+static void xennet_mark_tx_pending(struct netfront_queue *queue)
+{
+	unsigned int i;
+
+	while ((i = get_id_from_list(&queue->tx_pend_queue, queue->tx_link)) !=
+		TX_LINK_NONE)
+		queue->tx_link[i] = TX_PENDING;
+}
+
 #define MAX_XEN_SKB_FRAGS (65536 / XEN_PAGE_SIZE + 1)
 
 static netdev_tx_t xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)
@@ -565,6 +610,8 @@ static netdev_tx_t xennet_start_xmit(str
 	/* Drop the packet if no queues are set up */
 	if (num_queues < 1)
 		goto drop;
+	if (unlikely(np->broken))
+		goto drop;
 	/* Determine which queue to transmit this SKB on */
 	queue_index = skb_get_queue_mapping(skb);
 	queue = &np->queues[queue_index];
@@ -668,6 +715,8 @@ static netdev_tx_t xennet_start_xmit(str
 	/* First request has the packet length. */
 	first_tx->size = skb->len;
 
+	xennet_mark_tx_pending(queue);
+
 	RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&queue->tx, notify);
 	if (notify)
 		notify_remote_via_irq(queue->tx_irq);
@@ -992,6 +1041,13 @@ static int xennet_poll(struct napi_struc
 	skb_queue_head_init(&tmpq);
 
 	rp = queue->rx.sring->rsp_prod;
+	if (RING_RESPONSE_PROD_OVERFLOW(&queue->rx, rp)) {
+		dev_alert(&dev->dev, "Illegal number of responses %u\n",
+			  rp - queue->rx.rsp_cons);
+		queue->info->broken = true;
+		spin_unlock(&queue->rx_lock);
+		return 0;
+	}
 	rmb(); /* Ensure we see queued responses up to 'rp'. */
 
 	i = queue->rx.rsp_cons;
@@ -1210,6 +1266,9 @@ static irqreturn_t xennet_tx_interrupt(i
 	struct netfront_queue *queue = dev_id;
 	unsigned long flags;
 
+	if (queue->info->broken)
+		return IRQ_HANDLED;
+
 	spin_lock_irqsave(&queue->tx_lock, flags);
 	xennet_tx_buf_gc(queue);
 	spin_unlock_irqrestore(&queue->tx_lock, flags);
@@ -1222,6 +1281,9 @@ static irqreturn_t xennet_rx_interrupt(i
 	struct netfront_queue *queue = dev_id;
 	struct net_device *dev = queue->info->netdev;
 
+	if (queue->info->broken)
+		return IRQ_HANDLED;
+
 	if (likely(netif_carrier_ok(dev) &&
 		   RING_HAS_UNCONSUMED_RESPONSES(&queue->rx)))
 		napi_schedule(&queue->napi);
@@ -1243,6 +1305,10 @@ static void xennet_poll_controller(struc
 	struct netfront_info *info = netdev_priv(dev);
 	unsigned int num_queues = dev->real_num_tx_queues;
 	unsigned int i;
+
+	if (info->broken)
+		return;
+
 	for (i = 0; i < num_queues; ++i)
 		xennet_interrupt(0, &info->queues[i]);
 }
@@ -1614,6 +1680,7 @@ static int xennet_init_queue(struct netf
 
 	/* Initialise tx_skb_freelist as a free chain containing every entry. */
 	queue->tx_skb_freelist = 0;
+	queue->tx_pend_queue = TX_LINK_NONE;
 	for (i = 0; i < NET_TX_RING_SIZE; i++) {
 		queue->tx_link[i] = i + 1;
 		queue->grant_tx_ref[i] = GRANT_INVALID_REF;
@@ -1824,6 +1891,9 @@ static int talk_to_netback(struct xenbus
 	if (info->queues)
 		xennet_destroy_queues(info);
 
+	/* For the case of a reconnect reset the "broken" indicator. */
+	info->broken = false;
+
 	err = xennet_create_queues(info, &num_queues);
 	if (err < 0) {
 		xenbus_dev_fatal(dev, err, "creating queues");



  parent reply	other threads:[~2021-11-29 22:15 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-29 18:17 [PATCH 4.19 00/69] 4.19.219-rc1 review Greg Kroah-Hartman
2021-11-29 18:17 ` [PATCH 4.19 01/69] USB: serial: option: add Telit LE910S1 0x9200 composition Greg Kroah-Hartman
2021-11-29 18:17 ` [PATCH 4.19 02/69] USB: serial: option: add Fibocom FM101-GL variants Greg Kroah-Hartman
2021-11-29 18:17 ` [PATCH 4.19 03/69] usb: dwc2: hcd_queue: Fix use of floating point literal Greg Kroah-Hartman
2021-11-29 18:17 ` [PATCH 4.19 04/69] usb: hub: Fix usb enumeration issue due to address0 race Greg Kroah-Hartman
2021-11-29 18:17 ` [PATCH 4.19 05/69] usb: hub: Fix locking issues with address0_mutex Greg Kroah-Hartman
2021-11-29 18:17 ` [PATCH 4.19 06/69] binder: fix test regression due to sender_euid change Greg Kroah-Hartman
2021-11-29 18:17 ` [PATCH 4.19 07/69] ALSA: ctxfi: Fix out-of-range access Greg Kroah-Hartman
2021-11-29 18:17 ` [PATCH 4.19 08/69] media: cec: copy sequence field for the reply Greg Kroah-Hartman
2021-11-29 18:17 ` [PATCH 4.19 09/69] HID: wacom: Use "Confidence" flag to prevent reporting invalid contacts Greg Kroah-Hartman
2021-11-29 18:17 ` [PATCH 4.19 10/69] staging: rtl8192e: Fix use after free in _rtl92e_pci_disconnect() Greg Kroah-Hartman
2021-11-29 18:17 ` [PATCH 4.19 11/69] fuse: fix page stealing Greg Kroah-Hartman
2021-11-29 18:17 ` [PATCH 4.19 12/69] xen: dont continue xenstore initialization in case of errors Greg Kroah-Hartman
2021-11-29 18:17 ` [PATCH 4.19 13/69] xen: detect uninitialized xenbus in xenbus_init Greg Kroah-Hartman
2021-11-29 18:17 ` [PATCH 4.19 14/69] tracing: Fix pid filtering when triggers are attached Greg Kroah-Hartman
2021-11-29 18:17 ` [PATCH 4.19 15/69] xtensa: use CONFIG_USE_OF instead of CONFIG_OF Greg Kroah-Hartman
2021-11-29 18:17 ` [PATCH 4.19 16/69] proc/vmcore: fix clearing user buffer by properly using clear_user() Greg Kroah-Hartman
2021-11-29 18:17 ` [PATCH 4.19 17/69] PCI: aardvark: Fix a leaked reference by adding missing of_node_put() Greg Kroah-Hartman
2021-11-29 18:17   ` Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 18/69] PCI: aardvark: Wait for endpoint to be ready before training link Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 19/69] PCI: aardvark: Train link immediately after enabling training Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 20/69] PCI: aardvark: Improve link training Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 21/69] PCI: aardvark: Issue PERST via GPIO Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 22/69] PCI: aardvark: Replace custom macros by standard linux/pci_regs.h macros Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 23/69] PCI: aardvark: Indicate error in val when config read fails Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 24/69] PCI: aardvark: Dont touch PCIe registers if no card connected Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 25/69] PCI: aardvark: Fix compilation on s390 Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 26/69] PCI: aardvark: Move PCIe reset card code to advk_pcie_train_link() Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 27/69] PCI: aardvark: Update comment about disabling link training Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 28/69] PCI: aardvark: Configure PCIe resources from ranges DT property Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 29/69] PCI: aardvark: Fix PCIe Max Payload Size setting Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 30/69] PCI: aardvark: Fix link training Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 31/69] PCI: aardvark: Fix checking for link up via LTSSM state Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 32/69] pinctrl: armada-37xx: Correct mpp definitions Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 33/69] pinctrl: armada-37xx: add missing pin: PCIe1 Wakeup Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 34/69] pinctrl: armada-37xx: Correct PWM pins definitions Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 35/69] arm64: dts: marvell: armada-37xx: declare PCIe reset pin Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 36/69] arm64: dts: marvell: armada-37xx: Set pcie_reset_pin to gpio function Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 37/69] netfilter: ipvs: Fix reuse connection if RS weight is 0 Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 38/69] ARM: dts: BCM5301X: Fix I2C controller interrupt Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 39/69] ARM: dts: BCM5301X: Add interrupt properties to GPIO node Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 40/69] ASoC: qdsp6: q6routing: Conditionally reset FrontEnd Mixer Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 41/69] ASoC: topology: Add missing rwsem around snd_ctl_remove() calls Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 42/69] net: ieee802154: handle iftypes as u32 Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 43/69] firmware: arm_scmi: pm: Propagate return value to caller Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 44/69] NFSv42: Dont fail clone() unless the OP_CLONE operation failed Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 45/69] ARM: socfpga: Fix crash with CONFIG_FORTIRY_SOURCE Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 46/69] scsi: mpt3sas: Fix kernel panic during drive powercycle test Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 47/69] drm/vc4: fix error code in vc4_create_object() Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 48/69] ipv6: fix typos in __ip6_finish_output() Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 49/69] net/smc: Ensure the active closing peer first closes clcsock Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 50/69] PM: hibernate: use correct mode for swsusp_close() Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 51/69] tcp_cubic: fix spurious Hystart ACK train detections for not-cwnd-limited flows Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 52/69] MIPS: use 3-level pgtable for 64KB page size on MIPS_VA_BITS_48 Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 53/69] net/smc: Dont call clcsock shutdown twice when smc shutdown Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 54/69] net: hns3: fix VF RSS failed problem after PF enable multi-TCs Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 55/69] vhost/vsock: fix incorrect used length reported to the guest Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 56/69] tracing: Check pid filtering when creating events Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 57/69] s390/mm: validate VMA in PGSTE manipulation functions Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 58/69] hugetlbfs: flush TLBs correctly after huge_pmd_unshare Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 59/69] NFC: add NCI_UNREG flag to eliminate the race Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 60/69] fuse: release pipe buf after last use Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 61/69] xen: sync include/xen/interface/io/ring.h with Xens newest version Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 62/69] xen/blkfront: read response from backend only once Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 63/69] xen/blkfront: dont take local copy of a request from the ring page Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 64/69] xen/blkfront: dont trust the backend response data blindly Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 65/69] xen/netfront: read response from backend only once Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 66/69] xen/netfront: dont read data from request on the ring page Greg Kroah-Hartman
2021-11-29 18:18 ` [PATCH 4.19 67/69] xen/netfront: disentangle tx_skb_freelist Greg Kroah-Hartman
2021-11-29 18:18 ` Greg Kroah-Hartman [this message]
2021-11-29 18:18 ` [PATCH 4.19 69/69] tty: hvc: replace BUG_ON() with negative return value Greg Kroah-Hartman
2021-11-30  1:04 ` [PATCH 4.19 00/69] 4.19.219-rc1 review Shuah Khan
2021-11-30  1:24 ` Samuel Zou
2021-11-30  8:42 ` Jon Hunter
2021-11-30  9:32 ` Naresh Kamboju
2021-11-30 13:38 ` Sudip Mukherjee
2021-11-30 16:02 ` Pavel Machek
2021-11-30 17:42 ` Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211129181705.862563937@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=davem@davemloft.net \
    --cc=jbeulich@suse.com \
    --cc=jgross@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.