[Intel-wired-lan] [PATCH net] i40e: Cleanup PTP upon probe failure

[Intel-wired-lan] [PATCH net] i40e: Cleanup PTP upon probe failure

[Matt Vollrath]

Fix two conditions which would leak PTP registration on probe failure:

1. i40e_setup_pf_switch can encounter an error in
    i40e_setup_pf_filter_control, call i40e_ptp_init, then return
    non-zero, sending i40e_probe to err_vsis.

2. i40e_setup_misc_vector can return non-zero, sending i40e_probe to
    err_vsis.

Found with coccinelle.

Signed-off-by: Matt Vollrath <tactii@gmail.com>
---
  drivers/net/ethernet/intel/i40e/i40e_main.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 598739220dfb..dacdf5fad2ea 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -16112,6 +16112,7 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
  	/* Unwind what we've done if something failed in the setup */
  err_vsis:
  	set_bit(__I40E_DOWN, pf->state);
+	i40e_ptp_stop(pf);
  	i40e_clear_interrupt_scheme(pf);
  	kfree(pf->vsi);
  err_switch_setup:
-- 
2.43.0

Re: [Intel-wired-lan] [PATCH net] i40e: Cleanup PTP upon probe failure

[Kohei Enju]

On 04/04 11:19, Matt Vollrath wrote:
> Fix two conditions which would leak PTP registration on probe failure:
> 
> 1. i40e_setup_pf_switch can encounter an error in
>    i40e_setup_pf_filter_control, call i40e_ptp_init, then return
>    non-zero, sending i40e_probe to err_vsis.
> 
> 2. i40e_setup_misc_vector can return non-zero, sending i40e_probe to
>    err_vsis.
> 
> Found with coccinelle.
> 

Isn't a Fixes: tag needed?

> Signed-off-by: Matt Vollrath <tactii@gmail.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 598739220dfb..dacdf5fad2ea 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -16112,6 +16112,7 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	/* Unwind what we've done if something failed in the setup */
>  err_vsis:
>  	set_bit(__I40E_DOWN, pf->state);
> +	i40e_ptp_stop(pf);

Perhaps this is a separate issue from this one, but if i40e_probe()
fails and jumps to err_switch_setup, doesn't pf->ptp_pins leak?

Also, maybe because I'm not very familiar with this driver, I don't
understand why i40e_ptp_alloc_pins() is called this early, outside of
i40e_ptp_init(). It seems to make the error-path handling a bit more
complicated.

>  	i40e_clear_interrupt_scheme(pf);
>  	kfree(pf->vsi);
>  err_switch_setup:
> -- 
> 2.43.0

This diff seems mangled, and doesn't apply to my local trees.

Email header says:
  User-Agent: Mozilla Thunderbird
  ...
  Content-Type: text/plain; charset=UTF-8; format=flowed

It may be worth disabling format=flowed if you use Thunderbird.
See also: https://docs.kernel.org/process/email-clients.html

Re: [Intel-wired-lan] [PATCH net] i40e: Cleanup PTP upon probe failure

[Matt Vollrath]

On 4/5/26 13:03, Kohei Enju wrote:
> Isn't a Fixes: tag needed?

I will do the forensics and find where at least one of these conditions
was introduced.

> Perhaps this is a separate issue from this one, but if i40e_probe()
> fails and jumps to err_switch_setup, doesn't pf->ptp_pins leak?
> 
> Also, maybe because I'm not very familiar with this driver, I don't
> understand why i40e_ptp_alloc_pins() is called this early, outside of
> i40e_ptp_init(). It seems to make the error-path handling a bit more
> complicated.

I will look at this, and if I can fix it by adding a pin teardown call
to the probe exit path, I will add it.

I don't have hardware to test this with, so I am treading lightly and
not changing anything about the order of operations on the way up.

> This diff seems mangled, and doesn't apply to my local trees.
> 
> Email header says:
>    User-Agent: Mozilla Thunderbird
>    ...
>    Content-Type: text/plain; charset=UTF-8; format=flowed
> 
> It may be worth disabling format=flowed if you use Thunderbird.
> See also: https://docs.kernel.org/process/email-clients.html

It seems like setting up git for email will be the path of least
resistance for patches, so I will do that.

Thank you for the feedback.
mv