All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiasheng Jiang <jiasheng@iscas.ac.cn>
To: ecree.xilinx@gmail.com, habetsm.xilinx@gmail.com,
	davem@davemloft.net, kuba@kernel.org, bhelgaas@google.com,
	hkallweit1@gmail.com
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Jiasheng Jiang <jiasheng@iscas.ac.cn>
Subject: [PATCH] sfc: falcon: potential dereference null pointer of rx_queue->page_ring
Date: Fri, 17 Dec 2021 17:14:29 +0800	[thread overview]
Message-ID: <20211217091430.588034-1-jiasheng@iscas.ac.cn> (raw)

The return value of kcalloc() needs to be checked.
To avoid dereference of null pointer in case of the failure of alloc.
Therefore, it might be better to change the return type of
ef4_init_rx_recycle_ring(), ef4_init_rx_queue(), ef4_start_datapath(),
ef4_start_all(), and return -ENOMEM when alloc fails and return 0 the
others.
Also, ef4_realloc_channels(), ef4_net_open(), ef4_change_mtu(),
ef4_reset_up() and ef4_pm_thaw() should deal with the return value of
ef4_start_all().

Fixes: 5a6681e22c14 ("sfc: separate out SFC4000 ("Falcon") support into new sfc-falcon driver")
Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
---
 drivers/net/ethernet/sfc/falcon/efx.c | 46 ++++++++++++++++++++-------
 drivers/net/ethernet/sfc/falcon/efx.h |  2 +-
 drivers/net/ethernet/sfc/falcon/rx.c  | 18 ++++++++---
 3 files changed, 50 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/sfc/falcon/efx.c b/drivers/net/ethernet/sfc/falcon/efx.c
index 5e7a57b680ca..fbd55029988e 100644
--- a/drivers/net/ethernet/sfc/falcon/efx.c
+++ b/drivers/net/ethernet/sfc/falcon/efx.c
@@ -201,7 +201,7 @@ static void ef4_init_napi_channel(struct ef4_channel *channel);
 static void ef4_fini_napi(struct ef4_nic *efx);
 static void ef4_fini_napi_channel(struct ef4_channel *channel);
 static void ef4_fini_struct(struct ef4_nic *efx);
-static void ef4_start_all(struct ef4_nic *efx);
+static int ef4_start_all(struct ef4_nic *efx);
 static void ef4_stop_all(struct ef4_nic *efx);
 
 #define EF4_ASSERT_RESET_SERIALISED(efx)		\
@@ -590,7 +590,7 @@ static int ef4_probe_channels(struct ef4_nic *efx)
  * to propagate configuration changes (mtu, checksum offload), or
  * to clear hardware error conditions
  */
-static void ef4_start_datapath(struct ef4_nic *efx)
+static int ef4_start_datapath(struct ef4_nic *efx)
 {
 	netdev_features_t old_features = efx->net_dev->features;
 	bool old_rx_scatter = efx->rx_scatter;
@@ -598,6 +598,7 @@ static void ef4_start_datapath(struct ef4_nic *efx)
 	struct ef4_rx_queue *rx_queue;
 	struct ef4_channel *channel;
 	size_t rx_buf_len;
+	int ret;
 
 	/* Calculate the rx buffer allocation parameters required to
 	 * support the current MTU, including padding for header
@@ -668,7 +669,10 @@ static void ef4_start_datapath(struct ef4_nic *efx)
 		}
 
 		ef4_for_each_channel_rx_queue(rx_queue, channel) {
-			ef4_init_rx_queue(rx_queue);
+			ret = ef4_init_rx_queue(rx_queue);
+			if (ret)
+				return ret;
+
 			atomic_inc(&efx->active_queues);
 			ef4_stop_eventq(channel);
 			ef4_fast_push_rx_descriptors(rx_queue, false);
@@ -680,6 +684,7 @@ static void ef4_start_datapath(struct ef4_nic *efx)
 
 	if (netif_device_present(efx->net_dev))
 		netif_tx_wake_all_queues(efx->net_dev);
+	return 0;
 }
 
 static void ef4_stop_datapath(struct ef4_nic *efx)
@@ -853,7 +858,10 @@ ef4_realloc_channels(struct ef4_nic *efx, u32 rxq_entries, u32 txq_entries)
 			  "unable to restart interrupts on channel reallocation\n");
 		ef4_schedule_reset(efx, RESET_TYPE_DISABLE);
 	} else {
-		ef4_start_all(efx);
+		rc = ef4_start_all(efx);
+		if (rc)
+			return rc;
+
 		netif_device_attach(efx->net_dev);
 	}
 	return rc;
@@ -1814,8 +1822,10 @@ static int ef4_probe_all(struct ef4_nic *efx)
  * is safe to call multiple times, so long as the NIC is not disabled.
  * Requires the RTNL lock.
  */
-static void ef4_start_all(struct ef4_nic *efx)
+static int ef4_start_all(struct ef4_nic *efx)
 {
+	int ret;
+
 	EF4_ASSERT_RESET_SERIALISED(efx);
 	BUG_ON(efx->state == STATE_DISABLED);
 
@@ -1823,10 +1833,12 @@ static void ef4_start_all(struct ef4_nic *efx)
 	 * of these flags are safe to read under just the rtnl lock */
 	if (efx->port_enabled || !netif_running(efx->net_dev) ||
 	    efx->reset_pending)
-		return;
+		return 0;
 
 	ef4_start_port(efx);
-	ef4_start_datapath(efx);
+	ret = ef4_start_datapath(efx);
+	if (ret)
+		return ret;
 
 	/* Start the hardware monitor if there is one */
 	if (efx->type->monitor != NULL)
@@ -1838,6 +1850,8 @@ static void ef4_start_all(struct ef4_nic *efx)
 	spin_lock_bh(&efx->stats_lock);
 	efx->type->update_stats(efx, NULL, NULL);
 	spin_unlock_bh(&efx->stats_lock);
+
+	return 0;
 }
 
 /* Quiesce the hardware and software data path, and regular activity
@@ -2074,7 +2088,10 @@ int ef4_net_open(struct net_device *net_dev)
 	 * before the monitor starts running */
 	ef4_link_status_changed(efx);
 
-	ef4_start_all(efx);
+	rc = ef4_start_all(efx);
+	if (rc)
+		return rc;
+
 	ef4_selftest_async_start(efx);
 	return 0;
 }
@@ -2140,7 +2157,10 @@ static int ef4_change_mtu(struct net_device *net_dev, int new_mtu)
 	ef4_mac_reconfigure(efx);
 	mutex_unlock(&efx->mac_lock);
 
-	ef4_start_all(efx);
+	rc = ef4_start_all(efx);
+	if (rc)
+		return rc;
+
 	netif_device_attach(efx->net_dev);
 	return 0;
 }
@@ -2409,7 +2429,9 @@ int ef4_reset_up(struct ef4_nic *efx, enum reset_type method, bool ok)
 
 	mutex_unlock(&efx->mac_lock);
 
-	ef4_start_all(efx);
+	rc = ef4_start_all(efx);
+	if (rc)
+		return rc;
 
 	return 0;
 
@@ -3033,7 +3055,9 @@ static int ef4_pm_thaw(struct device *dev)
 		efx->phy_op->reconfigure(efx);
 		mutex_unlock(&efx->mac_lock);
 
-		ef4_start_all(efx);
+		rc = ef4_start_all(efx);
+		if (rc)
+			return rc;
 
 		netif_device_attach(efx->net_dev);
 
diff --git a/drivers/net/ethernet/sfc/falcon/efx.h b/drivers/net/ethernet/sfc/falcon/efx.h
index d3b4646545fa..483501b42667 100644
--- a/drivers/net/ethernet/sfc/falcon/efx.h
+++ b/drivers/net/ethernet/sfc/falcon/efx.h
@@ -39,7 +39,7 @@ void ef4_set_default_rx_indir_table(struct ef4_nic *efx);
 void ef4_rx_config_page_split(struct ef4_nic *efx);
 int ef4_probe_rx_queue(struct ef4_rx_queue *rx_queue);
 void ef4_remove_rx_queue(struct ef4_rx_queue *rx_queue);
-void ef4_init_rx_queue(struct ef4_rx_queue *rx_queue);
+int ef4_init_rx_queue(struct ef4_rx_queue *rx_queue);
 void ef4_fini_rx_queue(struct ef4_rx_queue *rx_queue);
 void ef4_fast_push_rx_descriptors(struct ef4_rx_queue *rx_queue, bool atomic);
 void ef4_rx_slow_fill(struct timer_list *t);
diff --git a/drivers/net/ethernet/sfc/falcon/rx.c b/drivers/net/ethernet/sfc/falcon/rx.c
index 966f13e7475d..cf9a4c387dd5 100644
--- a/drivers/net/ethernet/sfc/falcon/rx.c
+++ b/drivers/net/ethernet/sfc/falcon/rx.c
@@ -709,8 +709,8 @@ int ef4_probe_rx_queue(struct ef4_rx_queue *rx_queue)
 	return rc;
 }
 
-static void ef4_init_rx_recycle_ring(struct ef4_nic *efx,
-				     struct ef4_rx_queue *rx_queue)
+static int ef4_init_rx_recycle_ring(struct ef4_nic *efx,
+				    struct ef4_rx_queue *rx_queue)
 {
 	unsigned int bufs_in_recycle_ring, page_ring_size;
 
@@ -728,13 +728,19 @@ static void ef4_init_rx_recycle_ring(struct ef4_nic *efx,
 					    efx->rx_bufs_per_page);
 	rx_queue->page_ring = kcalloc(page_ring_size,
 				      sizeof(*rx_queue->page_ring), GFP_KERNEL);
+	if (!rx_queue->page_ring)
+		return -ENOMEM;
+
 	rx_queue->page_ptr_mask = page_ring_size - 1;
+
+	return 0;
 }
 
-void ef4_init_rx_queue(struct ef4_rx_queue *rx_queue)
+int ef4_init_rx_queue(struct ef4_rx_queue *rx_queue)
 {
 	struct ef4_nic *efx = rx_queue->efx;
 	unsigned int max_fill, trigger, max_trigger;
+	int ret;
 
 	netif_dbg(rx_queue->efx, drv, rx_queue->efx->net_dev,
 		  "initialising RX queue %d\n", ef4_rx_queue_index(rx_queue));
@@ -744,7 +750,9 @@ void ef4_init_rx_queue(struct ef4_rx_queue *rx_queue)
 	rx_queue->notified_count = 0;
 	rx_queue->removed_count = 0;
 	rx_queue->min_fill = -1U;
-	ef4_init_rx_recycle_ring(efx, rx_queue);
+	ret = ef4_init_rx_recycle_ring(efx, rx_queue);
+	if (ret)
+		return ret;
 
 	rx_queue->page_remove = 0;
 	rx_queue->page_add = rx_queue->page_ptr_mask + 1;
@@ -770,6 +778,8 @@ void ef4_init_rx_queue(struct ef4_rx_queue *rx_queue)
 
 	/* Set up RX descriptor ring */
 	ef4_nic_init_rx(rx_queue);
+
+	return 0;
 }
 
 void ef4_fini_rx_queue(struct ef4_rx_queue *rx_queue)
-- 
2.25.1


             reply	other threads:[~2021-12-17  9:15 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-17  9:14 Jiasheng Jiang [this message]
2021-12-17  9:32 ` [PATCH] sfc: falcon: potential dereference null pointer of rx_queue->page_ring Heiner Kallweit

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=20211217091430.588034-1-jiasheng@iscas.ac.cn \
    --to=jiasheng@iscas.ac.cn \
    --cc=bhelgaas@google.com \
    --cc=davem@davemloft.net \
    --cc=ecree.xilinx@gmail.com \
    --cc=habetsm.xilinx@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@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.