From 5aec55b46c6238506cdf0c60cd0e42ab77a1e5e0 Mon Sep 17 00:00:00 2001 From: Catherine Sullivan Date: Mon, 17 May 2021 14:08:11 -0700 Subject: [PATCH 1/5] gve: Check TX QPL was actually assigned Correctly check the TX QPL was assigned and unassigned if other steps in the allocation fail. Fixes: f5cedc84a30d (gve: Add transmit and receive support) Signed-off-by: Catherine Sullivan Signed-off-by: David Awogbemila Acked-by: Willem de Bruijn Signed-off-by: David S. Miller --- drivers/net/ethernet/google/gve/gve_tx.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/google/gve/gve_tx.c b/drivers/net/ethernet/google/gve/gve_tx.c index 6938f3a939d6..bb57c42872b4 100644 --- a/drivers/net/ethernet/google/gve/gve_tx.c +++ b/drivers/net/ethernet/google/gve/gve_tx.c @@ -212,10 +212,11 @@ static int gve_tx_alloc_ring(struct gve_priv *priv, int idx) tx->dev = &priv->pdev->dev; if (!tx->raw_addressing) { tx->tx_fifo.qpl = gve_assign_tx_qpl(priv); - + if (!tx->tx_fifo.qpl) + goto abort_with_desc; /* map Tx FIFO */ if (gve_tx_fifo_init(priv, &tx->tx_fifo)) - goto abort_with_desc; + goto abort_with_qpl; } tx->q_resources = @@ -236,6 +237,9 @@ static int gve_tx_alloc_ring(struct gve_priv *priv, int idx) abort_with_fifo: if (!tx->raw_addressing) gve_tx_fifo_release(priv, &tx->tx_fifo); +abort_with_qpl: + if (!tx->raw_addressing) + gve_unassign_qpl(priv, tx->tx_fifo.qpl->id); abort_with_desc: dma_free_coherent(hdev, bytes, tx->desc, tx->bus); tx->desc = NULL; From e96b491a0ffa35a8a9607c193fa4d894ca9fb32f Mon Sep 17 00:00:00 2001 From: David Awogbemila Date: Mon, 17 May 2021 14:08:12 -0700 Subject: [PATCH 2/5] gve: Update mgmt_msix_idx if num_ntfy changes If we do not get the expected number of vectors from pci_enable_msix_range, we update priv->num_ntfy_blks but not priv->mgmt_msix_idx. This patch fixes this so that priv->mgmt_msix_idx is updated accordingly. Fixes: f5cedc84a30d ("gve: Add transmit and receive support") Signed-off-by: David Awogbemila Acked-by: Willem de Bruijn Signed-off-by: David S. Miller --- drivers/net/ethernet/google/gve/gve_main.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c index 7302498c6df3..64192942ca53 100644 --- a/drivers/net/ethernet/google/gve/gve_main.c +++ b/drivers/net/ethernet/google/gve/gve_main.c @@ -220,6 +220,7 @@ static int gve_alloc_notify_blocks(struct gve_priv *priv) int vecs_left = new_num_ntfy_blks % 2; priv->num_ntfy_blks = new_num_ntfy_blks; + priv->mgmt_msix_idx = priv->num_ntfy_blks; priv->tx_cfg.max_queues = min_t(int, priv->tx_cfg.max_queues, vecs_per_type); priv->rx_cfg.max_queues = min_t(int, priv->rx_cfg.max_queues, From 5218e919c8d06279884aa0baf76778a6817d5b93 Mon Sep 17 00:00:00 2001 From: David Awogbemila Date: Mon, 17 May 2021 14:08:13 -0700 Subject: [PATCH 3/5] gve: Add NULL pointer checks when freeing irqs. When freeing notification blocks, we index priv->msix_vectors. If we failed to allocate priv->msix_vectors (see abort_with_msix_vectors) this could lead to a NULL pointer dereference if the driver is unloaded. Fixes: 893ce44df565 ("gve: Add basic driver framework for Compute Engine Virtual NIC") Signed-off-by: David Awogbemila Acked-by: Willem de Brujin Signed-off-by: David S. Miller --- drivers/net/ethernet/google/gve/gve_main.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c index 64192942ca53..21a5d058dab4 100644 --- a/drivers/net/ethernet/google/gve/gve_main.c +++ b/drivers/net/ethernet/google/gve/gve_main.c @@ -301,20 +301,22 @@ static void gve_free_notify_blocks(struct gve_priv *priv) { int i; - /* Free the irqs */ - for (i = 0; i < priv->num_ntfy_blks; i++) { - struct gve_notify_block *block = &priv->ntfy_blocks[i]; - int msix_idx = i; + if (priv->msix_vectors) { + /* Free the irqs */ + for (i = 0; i < priv->num_ntfy_blks; i++) { + struct gve_notify_block *block = &priv->ntfy_blocks[i]; + int msix_idx = i; - irq_set_affinity_hint(priv->msix_vectors[msix_idx].vector, - NULL); - free_irq(priv->msix_vectors[msix_idx].vector, block); + irq_set_affinity_hint(priv->msix_vectors[msix_idx].vector, + NULL); + free_irq(priv->msix_vectors[msix_idx].vector, block); + } + free_irq(priv->msix_vectors[priv->mgmt_msix_idx].vector, priv); } dma_free_coherent(&priv->pdev->dev, priv->num_ntfy_blks * sizeof(*priv->ntfy_blocks), priv->ntfy_blocks, priv->ntfy_block_bus); priv->ntfy_blocks = NULL; - free_irq(priv->msix_vectors[priv->mgmt_msix_idx].vector, priv); pci_disable_msix(priv->pdev); kvfree(priv->msix_vectors); priv->msix_vectors = NULL; From f81781835f0adfae8d701545386030d223efcd6f Mon Sep 17 00:00:00 2001 From: Catherine Sullivan Date: Mon, 17 May 2021 14:08:14 -0700 Subject: [PATCH 4/5] gve: Upgrade memory barrier in poll routine As currently written, if the driver checks for more work (via gve_tx_poll or gve_rx_poll) before the device posts work and the irq doorbell is not unmasked (via iowrite32be(GVE_IRQ_ACK | GVE_IRQ_EVENT, ...)) before the device attempts to raise an interrupt, an interrupt is lost and this could potentially lead to the traffic being completely halted. For example, if a tx queue has already been stopped, the driver won't get the chance to complete work and egress will be halted. We need a full memory barrier in the poll routine to ensure that the irq doorbell is unmasked before the driver checks for more work. Fixes: f5cedc84a30d ("gve: Add transmit and receive support") Signed-off-by: Catherine Sullivan Signed-off-by: David Awogbemila Acked-by: Willem de Brujin Signed-off-by: David S. Miller --- drivers/net/ethernet/google/gve/gve_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c index 21a5d058dab4..bbc423e93122 100644 --- a/drivers/net/ethernet/google/gve/gve_main.c +++ b/drivers/net/ethernet/google/gve/gve_main.c @@ -180,7 +180,7 @@ static int gve_napi_poll(struct napi_struct *napi, int budget) /* Double check we have no extra work. * Ensure unmask synchronizes with checking for work. */ - dma_rmb(); + mb(); if (block->tx) reschedule |= gve_tx_poll(block, -1); if (block->rx) From fbd4a28b4fa66faaa7f510c0adc531d37e0a7848 Mon Sep 17 00:00:00 2001 From: David Awogbemila Date: Mon, 17 May 2021 14:08:15 -0700 Subject: [PATCH 5/5] gve: Correct SKB queue index validation. SKBs with skb_get_queue_mapping(skb) == tx_cfg.num_queues should also be considered invalid. Fixes: f5cedc84a30d ("gve: Add transmit and receive support") Signed-off-by: David Awogbemila Acked-by: Willem de Brujin Signed-off-by: David S. Miller --- drivers/net/ethernet/google/gve/gve_tx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/google/gve/gve_tx.c b/drivers/net/ethernet/google/gve/gve_tx.c index bb57c42872b4..3e04a3973d68 100644 --- a/drivers/net/ethernet/google/gve/gve_tx.c +++ b/drivers/net/ethernet/google/gve/gve_tx.c @@ -593,7 +593,7 @@ netdev_tx_t gve_tx(struct sk_buff *skb, struct net_device *dev) struct gve_tx_ring *tx; int nsegs; - WARN(skb_get_queue_mapping(skb) > priv->tx_cfg.num_queues, + WARN(skb_get_queue_mapping(skb) >= priv->tx_cfg.num_queues, "skb queue index out of range"); tx = &priv->tx[skb_get_queue_mapping(skb)]; if (unlikely(gve_maybe_stop_tx(tx, skb))) {