Re: [PATCH net] ibmvnic fix NULL tx_pools and rx_tools issue at do_reset

[Brian W Hart]

On Mon, Aug 24, 2020 at 07:49:23PM -0400, Dany Madden wrote:
> From: Mingming Cao 
> 
> At the time of do_reset, ibmvnic tries to re-initalize the tx_pools
> and rx_pools to avoid re-allocating the long term buffer. However
> there is a window inside do_reset that the tx_pools and
> rx_pools were freed before re-initialized making it possible to deference
> null pointers.
> 
> This patch fixes this issue by checking that the tx_pool
> and rx_pool are not NULL after ibmvnic_login. If so, re-allocating
> the pools. This will avoid getting into calling reset_tx/rx_pools with
> NULL adapter tx_pools/rx_pools pointer. Also add null pointer check in
> reset_tx_pools and reset_rx_pools to safe handle NULL pointer case.
> 
> Signed-off-by: Mingming Cao 
> Signed-off-by: Dany Madden 
> ---
>  drivers/net/ethernet/ibm/ibmvnic.c | 15 ++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
> b/drivers/net/ethernet/ibm/ibmvnic.c
> index 5afb3c9c52d2..5ff48e55308b 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -479,6 +479,9 @@ static int reset_rx_pools(struct ibmvnic_adapter *adapter)
>   int i, j, rc;
>   u64 *size_array;
> 
> + if (!adapter->tx_pool)
> + return -1;
> +

Should this one be testing rx_pool?

brian

>   size_array = (u64 *)((u8 *)(adapter->login_rsp_buf) +
>   be32_to_cpu(adapter->login_rsp_buf->off_rxadd_buff_size));
> 
> @@ -649,6 +652,9 @@ static int reset_tx_pools(struct ibmvnic_adapter *adapter)
>   int tx_scrqs;
>   int i, rc;
> 
> + if (!adapter->tx_pool)
> + return -1;
> +
>   tx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_txsubm_subcrqs);
>   for (i = 0; i < tx_scrqs; i++) {
>   rc = reset_one_tx_pool(adapter, >tso_pool[i]);
> @@ -2011,7 +2017,10 @@ static int do_reset(struct ibmvnic_adapter *adapter,
>   adapter->req_rx_add_entries_per_subcrq !=
>   old_num_rx_slots ||
>   adapter->req_tx_entries_per_subcrq !=
> - old_num_tx_slots) {
> + old_num_tx_slots ||
> + !adapter->rx_pool ||
> + !adapter->tso_pool ||
> + !adapter->tx_pool) {
>   release_rx_pools(adapter);
>   release_tx_pools(adapter);
>   release_napi(adapter);
> @@ -2024,10 +2033,14 @@ static int do_reset(struct ibmvnic_adapter *adapter,
>   } else {
>   rc = reset_tx_pools(adapter);
>   if (rc)
> + netdev_dbg(adapter->netdev, "reset tx pools 
> failed (%d)\n",
> + rc);
>   goto out;
> 
>   rc = reset_rx_pools(adapter);
>   if (rc)
> + netdev_dbg(adapter->netdev, "reset rx pools 
> failed (%d)\n",
> + rc);
>   goto out;
>   }
>   ibmvnic_disable_irqs(adapter);
> -- 
> 2.18.2
> 

[PATCH net] ibmvnic fix NULL tx_pools and rx_tools issue at do_reset

[Dany Madden]

From: Mingming Cao 

At the time of do_reset, ibmvnic tries to re-initalize the tx_pools
and rx_pools to avoid re-allocating the long term buffer. However
there is a window inside do_reset that the tx_pools and
rx_pools were freed before re-initialized making it possible to deference
null pointers.

This patch fixes this issue by checking that the tx_pool
and rx_pool are not NULL after ibmvnic_login. If so, re-allocating
the pools. This will avoid getting into calling reset_tx/rx_pools with
NULL adapter tx_pools/rx_pools pointer. Also add null pointer check in
reset_tx_pools and reset_rx_pools to safe handle NULL pointer case.

Signed-off-by: Mingming Cao 
Signed-off-by: Dany Madden 
---
 drivers/net/ethernet/ibm/ibmvnic.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 5afb3c9c52d2..5ff48e55308b 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -479,6 +479,9 @@ static int reset_rx_pools(struct ibmvnic_adapter *adapter)
int i, j, rc;
u64 *size_array;
 
+   if (!adapter->tx_pool)
+   return -1;
+
size_array = (u64 *)((u8 *)(adapter->login_rsp_buf) +
be32_to_cpu(adapter->login_rsp_buf->off_rxadd_buff_size));
 
@@ -649,6 +652,9 @@ static int reset_tx_pools(struct ibmvnic_adapter *adapter)
int tx_scrqs;
int i, rc;
 
+   if (!adapter->tx_pool)
+   return -1;
+
tx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_txsubm_subcrqs);
for (i = 0; i < tx_scrqs; i++) {
rc = reset_one_tx_pool(adapter, >tso_pool[i]);
@@ -2011,7 +2017,10 @@ static int do_reset(struct ibmvnic_adapter *adapter,
adapter->req_rx_add_entries_per_subcrq !=
old_num_rx_slots ||
adapter->req_tx_entries_per_subcrq !=
-   old_num_tx_slots) {
+   old_num_tx_slots ||
+   !adapter->rx_pool ||
+   !adapter->tso_pool ||
+   !adapter->tx_pool) {
release_rx_pools(adapter);
release_tx_pools(adapter);
release_napi(adapter);
@@ -2024,10 +2033,14 @@ static int do_reset(struct ibmvnic_adapter *adapter,
} else {
rc = reset_tx_pools(adapter);
if (rc)
+   netdev_dbg(adapter->netdev, "reset tx pools 
failed (%d)\n",
+   rc);
goto out;
 
rc = reset_rx_pools(adapter);
if (rc)
+   netdev_dbg(adapter->netdev, "reset rx pools 
failed (%d)\n",
+   rc);
goto out;
}
ibmvnic_disable_irqs(adapter);
-- 
2.18.2