[PATCH v3] ice: ice_adapter: release xa entry on adapter allocation failure

[PATCH v3] ice: ice_adapter: release xa entry on adapter allocation failure

[Haotian Zhang]

When ice_adapter_new() fails, the reserved XArray entry created by
xa_insert() is not released. This causes subsequent insertions at
the same index to return -EBUSY, potentially leading to
NULL pointer dereferences.

Reorder the operations as suggested by Przemek Kitszel:
1. Check if adapter already exists (xa_load)
2. Reserve the XArray slot (xa_reserve)
3. Allocate the adapter (ice_adapter_new)
4. Store the adapter (xa_store)

Fixes: 0f0023c649c7 ("ice: do not init struct ice_adapter more times than needed")
Suggested-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Suggested-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>

---
Changes in v3:
  - Reorder xa_load/xa_reserve/ice_adapter_new/xa_store calls as
    suggested by Przemek Kitszel, instead of just adding xa_release().
Changes in v2:
  - Instead of checking the return value of xa_store(), fix the real bug
    where a failed ice_adapter_new() would leave a stale entry in the
    XArray.
  - Use xa_release() to clean up the reserved entry, as suggested by
    Jacob Keller.
---
 drivers/net/ethernet/intel/ice/ice_adapter.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.c b/drivers/net/ethernet/intel/ice/ice_adapter.c
index b53561c34708..0a8a48cd4bce 100644
--- a/drivers/net/ethernet/intel/ice/ice_adapter.c
+++ b/drivers/net/ethernet/intel/ice/ice_adapter.c
@@ -99,19 +99,21 @@ struct ice_adapter *ice_adapter_get(struct pci_dev *pdev)
 
 	index = ice_adapter_xa_index(pdev);
 	scoped_guard(mutex, &ice_adapters_mutex) {
-		err = xa_insert(&ice_adapters, index, NULL, GFP_KERNEL);
-		if (err == -EBUSY) {
-			adapter = xa_load(&ice_adapters, index);
+		adapter = xa_load(&ice_adapters, index);
+		if (adapter) {
 			refcount_inc(&adapter->refcount);
 			WARN_ON_ONCE(adapter->index != ice_adapter_index(pdev));
 			return adapter;
 		}
+		err = xa_reserve(&ice_adapters, index, GFP_KERNEL);
 		if (err)
 			return ERR_PTR(err);
 
 		adapter = ice_adapter_new(pdev);
-		if (!adapter)
+		if (!adapter) {
+			xa_release(&ice_adapters, index);
 			return ERR_PTR(-ENOMEM);
+		}
 		xa_store(&ice_adapters, index, adapter, GFP_KERNEL);
 	}
 	return adapter;
-- 
2.50.1.windows.1

RE: [Intel-wired-lan] [PATCH v3] ice: ice_adapter: release xa entry on adapter allocation failure

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Haotian Zhang
> Sent: Wednesday, October 1, 2025 1:54 PM
> To: Keller, Jacob E <jacob.e.keller@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Cc: Andrew Lunn <andrew+netdev@lunn.ch>; David S . Miller
> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; intel-
> wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Haotian Zhang <vulab@iscas.ac.cn>
> Subject: [Intel-wired-lan] [PATCH v3] ice: ice_adapter: release xa
> entry on adapter allocation failure
> 
> When ice_adapter_new() fails, the reserved XArray entry created by
> xa_insert() is not released. This causes subsequent insertions at
> the same index to return -EBUSY, potentially leading to
> NULL pointer dereferences.
> 
> Reorder the operations as suggested by Przemek Kitszel:
> 1. Check if adapter already exists (xa_load)
> 2. Reserve the XArray slot (xa_reserve)
> 3. Allocate the adapter (ice_adapter_new)
> 4. Store the adapter (xa_store)
> 
> Fixes: 0f0023c649c7 ("ice: do not init struct ice_adapter more times
> than needed")
> Suggested-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Suggested-by: Jacob Keller <jacob.e.keller@intel.com>
> Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

> 
> ---
> Changes in v3:
>   - Reorder xa_load/xa_reserve/ice_adapter_new/xa_store calls as
>     suggested by Przemek Kitszel, instead of just adding xa_release().
> Changes in v2:
>   - Instead of checking the return value of xa_store(), fix the real
> bug
>     where a failed ice_adapter_new() would leave a stale entry in the
>     XArray.
>   - Use xa_release() to clean up the reserved entry, as suggested by
>     Jacob Keller.
> ---

...

> --
> 2.50.1.windows.1

Re: [PATCH v3] ice: ice_adapter: release xa entry on adapter allocation failure

[Jacob Keller]

[-- Attachment #1.1: Type: text/plain, Size: 3163 bytes --]



On 10/1/2025 4:53 AM, Haotian Zhang wrote:
> When ice_adapter_new() fails, the reserved XArray entry created by
> xa_insert() is not released. This causes subsequent insertions at
> the same index to return -EBUSY, potentially leading to
> NULL pointer dereferences.
> 
> Reorder the operations as suggested by Przemek Kitszel:
> 1. Check if adapter already exists (xa_load)
> 2. Reserve the XArray slot (xa_reserve)
> 3. Allocate the adapter (ice_adapter_new)
> 4. Store the adapter (xa_store)
> 
> Fixes: 0f0023c649c7 ("ice: do not init struct ice_adapter more times than needed")
> Suggested-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Suggested-by: Jacob Keller <jacob.e.keller@intel.com>
> Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
> 

Thanks. I think this flow is a bit easier to understand and everything
works well now.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

> ---
> Changes in v3:
>   - Reorder xa_load/xa_reserve/ice_adapter_new/xa_store calls as
>     suggested by Przemek Kitszel, instead of just adding xa_release().
> Changes in v2:
>   - Instead of checking the return value of xa_store(), fix the real bug
>     where a failed ice_adapter_new() would leave a stale entry in the
>     XArray.
>   - Use xa_release() to clean up the reserved entry, as suggested by
>     Jacob Keller.
> ---
>  drivers/net/ethernet/intel/ice/ice_adapter.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.c b/drivers/net/ethernet/intel/ice/ice_adapter.c
> index b53561c34708..0a8a48cd4bce 100644
> --- a/drivers/net/ethernet/intel/ice/ice_adapter.c
> +++ b/drivers/net/ethernet/intel/ice/ice_adapter.c
> @@ -99,19 +99,21 @@ struct ice_adapter *ice_adapter_get(struct pci_dev *pdev)
>  
>  	index = ice_adapter_xa_index(pdev);
>  	scoped_guard(mutex, &ice_adapters_mutex) {
> -		err = xa_insert(&ice_adapters, index, NULL, GFP_KERNEL);
> -		if (err == -EBUSY) {
> -			adapter = xa_load(&ice_adapters, index);
> +		adapter = xa_load(&ice_adapters, index);
> +		if (adapter) {
>  			refcount_inc(&adapter->refcount);
>  			WARN_ON_ONCE(adapter->index != ice_adapter_index(pdev));
>  			return adapter;
>  		}
> +		err = xa_reserve(&ice_adapters, index, GFP_KERNEL);
>  		if (err)
>  			return ERR_PTR(err);
>  
>  		adapter = ice_adapter_new(pdev);
> -		if (!adapter)
> +		if (!adapter) {
> +			xa_release(&ice_adapters, index);

Strictly we might not actually need xa_release now, because xa_load will
return NULL on a reserved entry, then xa_reserve will be a no-op if the
entry is already reserved, I believe, but I think its best to keep it
for clarity and because it frees up otherwise unused memory which seems
important since ice_adapter_new should only really fail if we're out of
memory. Additionally, this is an error path and not something that
happens every run so it is unlikely to be part of a performance critical
bottleneck.

Thanks!

>  			return ERR_PTR(-ENOMEM);
> +		}
>  		xa_store(&ice_adapters, index, adapter, GFP_KERNEL);
>  	}
>  	return adapter;


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH v3] ice: ice_adapter: release xa entry on adapter allocation failure

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed,  1 Oct 2025 19:53:36 +0800 you wrote:
> When ice_adapter_new() fails, the reserved XArray entry created by
> xa_insert() is not released. This causes subsequent insertions at
> the same index to return -EBUSY, potentially leading to
> NULL pointer dereferences.
> 
> Reorder the operations as suggested by Przemek Kitszel:
> 1. Check if adapter already exists (xa_load)
> 2. Reserve the XArray slot (xa_reserve)
> 3. Allocate the adapter (ice_adapter_new)
> 4. Store the adapter (xa_store)
> 
> [...]

Here is the summary with links:
  - [v3] ice: ice_adapter: release xa entry on adapter allocation failure
    https://git.kernel.org/netdev/net/c/2db687f3469d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html