netdev - Re: [PATCH iwl-net v2 5/5] idpf: fix error handling in the init

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <31f701e6-fbd3-4674-82ef-2f835d4a8b41@intel.com>
Date: Mon, 1 Dec 2025 13:44:01 -0800
From: "Chittim, Madhu" <madhu.chittim@...el.com>
To: "Tantilov, Emil S" <emil.s.tantilov@...el.com>,
	"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "Loktionov, Aleksandr"
	<Aleksandr.Loktionov@...el.com>, "Kitszel, Przemyslaw"
	<przemyslaw.kitszel@...el.com>, "Nguyen, Anthony L"
	<anthony.l.nguyen@...el.com>, "andrew+netdev@...n.ch"
	<andrew+netdev@...n.ch>, "davem@...emloft.net" <davem@...emloft.net>,
	"edumazet@...gle.com" <edumazet@...gle.com>, "kuba@...nel.org"
	<kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"decot@...gle.com" <decot@...gle.com>, "willemb@...gle.com"
	<willemb@...gle.com>, "Hay, Joshua A" <joshua.a.hay@...el.com>, "Lobakin,
 Aleksander" <aleksander.lobakin@...el.com>, "Zaremba, Larysa"
	<larysa.zaremba@...el.com>, "iamvivekkumar@...gle.com"
	<iamvivekkumar@...gle.com>
Subject: Re: [PATCH iwl-net v2 5/5] idpf: fix error handling in the init_task
 on load



On 11/20/2025 4:12 PM, Tantilov, Emil S wrote:
> If the init_task fails during a driver load, we end up without vports and
> netdevs, effectively failing the entire process. In that state a
> subsequent reset will result in a crash as the service task attempts to
> access uninitialized resources. Following trace is from an error in the
> init_task where the CREATE_VPORT (op 501) is rejected by the FW:
> 
> [40922.763136] idpf 0000:83:00.0: Device HW Reset initiated
> [40924.449797] idpf 0000:83:00.0: Transaction failed (op 501)
> [40958.148190] idpf 0000:83:00.0: HW reset detected
> [40958.161202] BUG: kernel NULL pointer dereference, address: 00000000000000a8
> ...
> [40958.168094] Workqueue: idpf-0000:83:00.0-vc_event idpf_vc_event_task [idpf]
> [40958.168865] RIP: 0010:idpf_vc_event_task+0x9b/0x350 [idpf]
> ...
> [40958.177932] Call Trace:
> [40958.178491]  <TASK>
> [40958.179040]  process_one_work+0x226/0x6d0
> [40958.179609]  worker_thread+0x19e/0x340
> [40958.180158]  ? __pfx_worker_thread+0x10/0x10
> [40958.180702]  kthread+0x10f/0x250
> [40958.181238]  ? __pfx_kthread+0x10/0x10
> [40958.181774]  ret_from_fork+0x251/0x2b0
> [40958.182307]  ? __pfx_kthread+0x10/0x10
> [40958.182834]  ret_from_fork_asm+0x1a/0x30
> [40958.183370]  </TASK>
> 
> Fix the error handling in the init_task to make sure the service and
> mailbox tasks are disabled if the error happens during load. These are
> started in idpf_vc_core_init(), which spawns the init_task and has no way
> of knowing if it failed. If the error happens on reset, following
> successful driver load, the tasks can still run, as that will allow the
> netdevs to attempt recovery through another reset. Stop the PTP callbacks
> either way as those will be restarted by the call to idpf_vc_core_init()
> during a successful reset.
> 
> Fixes: 0fe45467a104 ("idpf: add create vport and netdev configuration")
> Reported-by: Vivek Kumar <iamvivekkumar@...gle.com>
> Signed-off-by: Emil Tantilov <emil.s.tantilov@...el.com>

Reviewed-by: Madhu Chittim <madhu.chittim@...el.com>

> ---
>   drivers/net/ethernet/intel/idpf/idpf_lib.c | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c
> index 5193968c9bb1..89f3b46378c4 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
> @@ -1716,10 +1716,9 @@ void idpf_init_task(struct work_struct *work)
>   		set_bit(IDPF_VPORT_REG_NETDEV, vport_config->flags);
>   	}
>   
> -	/* As all the required vports are created, clear the reset flag
> -	 * unconditionally here in case we were in reset and the link was down.
> -	 */
> +	/* Clear the reset and load bits as all vports are created */
>   	clear_bit(IDPF_HR_RESET_IN_PROG, adapter->flags);
> +	clear_bit(IDPF_HR_DRV_LOAD, adapter->flags);
>   	/* Start the statistics task now */
>   	queue_delayed_work(adapter->stats_wq, &adapter->stats_task,
>   			   msecs_to_jiffies(10 * (pdev->devfn & 0x07)));
> @@ -1733,6 +1732,15 @@ void idpf_init_task(struct work_struct *work)
>   				idpf_vport_dealloc(adapter->vports[index]);
>   		}
>   	}
> +	/* Cleanup after vc_core_init, which has no way of knowing the
> +	 * init task failed on driver load.
> +	 */
> +	if (test_and_clear_bit(IDPF_HR_DRV_LOAD, adapter->flags)) {
> +		cancel_delayed_work_sync(&adapter->serv_task);
> +		cancel_delayed_work_sync(&adapter->mbx_task);
> +	}
> +	idpf_ptp_release(adapter);
> +
>   	clear_bit(IDPF_HR_RESET_IN_PROG, adapter->flags);
>   }
>   
> @@ -1882,7 +1890,7 @@ static void idpf_init_hard_reset(struct idpf_adapter *adapter)
>   	dev_info(dev, "Device HW Reset initiated\n");
>   
>   	/* Prepare for reset */
> -	if (test_and_clear_bit(IDPF_HR_DRV_LOAD, adapter->flags)) {
> +	if (test_bit(IDPF_HR_DRV_LOAD, adapter->flags)) {
>   		reg_ops->trigger_reset(adapter, IDPF_HR_DRV_LOAD);
>   	} else if (test_and_clear_bit(IDPF_HR_FUNC_RESET, adapter->flags)) {
>   		bool is_reset = idpf_is_reset_detected(adapter);