[Intel-wired-lan] [PATCH] i40e: fix NULL pointer dereference in i40e_lan_del_device()

Linkui Xiao posted 1 patch 1 week, 2 days ago
drivers/net/ethernet/intel/i40e/i40e_client.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
[Intel-wired-lan] [PATCH] i40e: fix NULL pointer dereference in i40e_lan_del_device()
Posted by Linkui Xiao 1 week, 2 days ago
From: Linkui Xiao <xiaolinkui@kylinos.cn>

i40e_lan_del_device() dereferences pf->cinst unconditionally. pf->cinst
is only assigned by i40e_client_add_instance(), which resets it to NULL
whenever the i40e_client_instance allocation, i40e_client_get_params()
or i40e_register_auxiliary_dev() fails, and it returns void so
i40e_lan_add_device() cannot report any of that to its caller.

i40e_lan_add_device() can also fail before it ever gets there, for
example when the i40e_device allocation fails, and i40e_probe() only
prints a message instead of clearing I40E_FLAG_IWARP_ENA. i40e_remove()
therefore still calls i40e_lan_del_device() and dereferences a NULL
pf->cinst.

Only tear the auxiliary device down when an instance was actually
created. Removing the PF from the i40e_devices list below does not
depend on it.

Fixes: f4370a85d62e ("i40e: Register auxiliary devices to provide RDMA")
Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
---
 drivers/net/ethernet/intel/i40e/i40e_client.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.c b/drivers/net/ethernet/intel/i40e/i40e_client.c
index 84a97ca8a6d8..9a4f50af5347 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_client.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_client.c
@@ -496,15 +496,22 @@ int i40e_lan_add_device(struct i40e_pf *pf)
  **/
 int i40e_lan_del_device(struct i40e_pf *pf)
 {
-	struct auxiliary_device *aux_dev = pf->cinst->lan_info.aux_dev;
+	struct auxiliary_device *aux_dev;
 	struct i40e_device *ldev, *tmp;
 	int ret = -ENODEV;
 
-	auxiliary_device_delete(aux_dev);
-	auxiliary_device_uninit(aux_dev);
+	/* i40e_client_add_instance() leaves pf->cinst NULL when it fails,
+	 * so there may be no auxiliary device to tear down here.
+	 */
+	if (pf->cinst) {
+		aux_dev = pf->cinst->lan_info.aux_dev;
 
-	/* First, remove any client instance. */
-	i40e_client_del_instance(pf);
+		auxiliary_device_delete(aux_dev);
+		auxiliary_device_uninit(aux_dev);
+
+		/* First, remove any client instance. */
+		i40e_client_del_instance(pf);
+	}
 
 	mutex_lock(&i40e_device_mutex);
 	list_for_each_entry_safe(ldev, tmp, &i40e_devices, list) {
-- 
2.25.1
RE: [Intel-wired-lan] [PATCH] i40e: fix NULL pointer dereference in i40e_lan_del_device()
Posted by Loktionov, Aleksandr 1 week, 2 days ago

> -----Original Message-----
> From: Linkui Xiao <xiaolinkui@126.com>
> Sent: Tuesday, September 15, 2026 12:55 PM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; andrew+netdev@lunn.ch;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Linkui Xiao <xiaolinkui@kylinos.cn>
> Subject: [Intel-wired-lan] [PATCH] i40e: fix NULL pointer dereference
> in i40e_lan_del_device()
> 
> From: Linkui Xiao <xiaolinkui@kylinos.cn>
> 
> i40e_lan_del_device() dereferences pf->cinst unconditionally. pf-
> >cinst is only assigned by i40e_client_add_instance(), which resets it
> to NULL whenever the i40e_client_instance allocation,
> i40e_client_get_params() or i40e_register_auxiliary_dev() fails, and
> it returns void so
> i40e_lan_add_device() cannot report any of that to its caller.
> 
> i40e_lan_add_device() can also fail before it ever gets there, for
> example when the i40e_device allocation fails, and i40e_probe() only
> prints a message instead of clearing I40E_FLAG_IWARP_ENA.
> i40e_remove() therefore still calls i40e_lan_del_device() and
> dereferences a NULL
> pf->cinst.
> 
> Only tear the auxiliary device down when an instance was actually
> created. Removing the PF from the i40e_devices list below does not
> depend on it.
> 
> Fixes: f4370a85d62e ("i40e: Register auxiliary devices to provide
> RDMA")
> Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_client.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.c
> b/drivers/net/ethernet/intel/i40e/i40e_client.c
> index 84a97ca8a6d8..9a4f50af5347 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_client.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_client.c
> @@ -496,15 +496,22 @@ int i40e_lan_add_device(struct i40e_pf *pf)
>   **/
>  int i40e_lan_del_device(struct i40e_pf *pf)  {
> -	struct auxiliary_device *aux_dev = pf->cinst->lan_info.aux_dev;
> +	struct auxiliary_device *aux_dev;
>  	struct i40e_device *ldev, *tmp;
>  	int ret = -ENODEV;
> 
> -	auxiliary_device_delete(aux_dev);
> -	auxiliary_device_uninit(aux_dev);
> +	/* i40e_client_add_instance() leaves pf->cinst NULL when it
> fails,
> +	 * so there may be no auxiliary device to tear down here.
> +	 */
> +	if (pf->cinst) {
> +		aux_dev = pf->cinst->lan_info.aux_dev;
> 
> -	/* First, remove any client instance. */
> -	i40e_client_del_instance(pf);
> +		auxiliary_device_delete(aux_dev);
> +		auxiliary_device_uninit(aux_dev);
> +
> +		/* First, remove any client instance. */
> +		i40e_client_del_instance(pf);
> +	}
> 
>  	mutex_lock(&i40e_device_mutex);
>  	list_for_each_entry_safe(ldev, tmp, &i40e_devices, list) {
> --
> 2.25.1

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>