Re: [PATCH v2 net-next 14/15] net: enetc: add PSI-to-VSI link status notification support for VF

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Race condition in VF message workqueue teardown leading to unhandled hardware interrupts.
--

commit 56c5e3ce01133d9549230bbfb6889bc307567cb5
Author: Wei Fang <wei.fang@nxp.com>

net: enetc: add PSI-to-VSI link status notification support for VF
    
This commit adds infrastructure for ENETC v4 VFs to track PF link status
changes via the PSI-to-VSI messaging channel. A dedicated MSI-X vector
handles incoming messages and schedules a work item to update the carrier
state.

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_vf.c b/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> index dc9a0f74a2e34..ba7686fb32ce9 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_vf.c
[ ... ]
> @@ -415,6 +461,122 @@ static void enetc_vf_netdev_setup(struct enetc_si *si, struct net_device *ndev,
[ ... ]
> +static void enetc_vf_msg_task(struct work_struct *work)
> +{
[ ... ]
> +	/* If ndev is in the NETREG_UNREGISTERED state, return directly
> +	 * without processing the message or enabling MR interrupt. Any
> +	 * future caller that reuses this work item after the device
> +	 * returns to a registered state should enable MR interrupt again.
> +	 */
> +	if (ndev->reg_state == NETREG_UNREGISTERED)
> +		return;
> +
[ ... ]
> +	enetc_vf_enable_mr_int(hw);
> +}

[Severity: High]
Can this task re-enable hardware interrupts after the OS IRQ handler has
been freed?

If this work item executes during a probe failure or races with
unregister_netdev() during remove, the ndev->reg_state check might not
protect it. During a probe failure, the state is NETREG_UNINITIALIZED, and
during remove, this check can race with the state change.

[ ... ]
> @@ -493,6 +683,18 @@ static int enetc_vf_probe(struct pci_dev *pdev,
[ ... ]
>  err_reg_netdev:
> +	enetc_vf_free_msg_msix(si);
> +err_register_msg_msix:
> +	enetc_vf_wq_task_destroy(si);
> +err_wq_init:
>  	enetc_free_msix(priv);

[Severity: High]
Can this teardown order cause an unhandled hardware interrupt?

If enetc_vf_free_msg_msix() frees the OS IRQ before enetc_vf_wq_task_destroy()
drains and disables the workqueue, the work task could run and unmask the
hardware interrupt while the handler is already gone.

Should enetc_vf_wq_task_destroy() be called before freeing the IRQ?

[ ... ]
> @@ -528,7 +734,8 @@ static void enetc_vf_remove(struct pci_dev *pdev)
>  
>  	priv = netdev_priv(si->ndev);
>  	unregister_netdev(si->ndev);
> -
> +	enetc_vf_free_msg_msix(si);
> +	enetc_vf_wq_task_destroy(si);

[Severity: High]
Similar to the probe error path, does freeing the IRQ before destroying
the workqueue allow the work item to execute and re-enable the hardware
interrupt after the handler is unregistered?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610091844.3423693-1-wei.fang@oss.nxp.com?part=14