Re: [PATCH net-next v08 3/9] hinic3: Add .ndo_tx_timeout and .ndo_get_stats64

[Jakub Kicinski <kuba@kernel.org>]

AI code review points out:

> @@ -329,6 +359,7 @@ static int hinic3_nic_probe(struct auxiliary_device *adev,
>  	if (err)
>  		goto err_uninit_sw;
>
> +	queue_delayed_work(nic_dev->workq, &nic_dev->periodic_work, HZ);
>  	netif_carrier_off(netdev);
>
>  	err = register_netdev(netdev);
> @@ -346,7 +377,8 @@ static int hinic3_nic_probe(struct auxiliary_device *adev,
>
>  err_free_nic_io:
>  	hinic3_free_nic_io(nic_dev);
> -
> +err_free_nic_dev:
> +	hinic3_free_nic_dev(nic_dev);

If register_netdev() fails after the delayed work has been queued, the error
path reaches hinic3_free_nic_dev() which calls destroy_workqueue() without
first cancelling the pending delayed work. The destroy_workqueue()
documentation states that all delayed_work must be cancelled before calling
it.

Should there be a disable_delayed_work_sync() or cancel_delayed_work_sync()
call in the error path before hinic3_free_nic_dev()? The remove path handles
this correctly:

> @@ -368,6 +400,9 @@ static void hinic3_nic_remove(struct auxiliary_device *adev)
>  	netdev = nic_dev->netdev;
>  	unregister_netdev(netdev);
>
> +	disable_delayed_work_sync(&nic_dev->periodic_work);
> +	hinic3_free_nic_dev(nic_dev);

Here in hinic3_nic_remove() the work is properly cancelled before destroying
the workqueue, but the probe error path after queue_delayed_work() does not
have similar protection.