[Intel-wired-lan] [Patch 1/2] iavf: Fix panic in iavf_remove

[Ken Cox <jkc@redhat.com>]

On 12/13/21 12:26, Nguyen, Anthony L wrote:
> On Wed, 2021-12-08 at 04:21 -0600, Ken Cox wrote:
>> It's possible for the client_task to get scheduled by the watchdog
>> after cancel_delayed_work_sync(&adapter->client_task);? This can
>> cause
>> a panic because free_netdev() is called with the client_task still
>> queued
>> on the work queue.
>>
>> The stack backtrace usually looks similar to:
>>
>> [? 121.272963] Workqueue:? 0x0 (iavf)
>> [? 121.272969] RIP: 0010:__list_del_entry_valid.cold.1+0x20/0x4c
>> ...
>> [? 121.272980] Call Trace:
>> [? 121.272985]? move_linked_works+0x49/0xa0
>> [? 121.272988]? pwq_activate_delayed_work+0x43/0x100
>> [? 121.272991]? pwq_dec_nr_in_flight+0x5d/0x90
>> [? 121.272993]? worker_thread+0x30/0x370
>> [? 121.272995]? ? process_one_work+0x420/0x420
>> [? 121.272998]? kthread+0x15d/0x180
>> [? 121.273000]? ? __kthread_parkme+0xa0/0xa0
>> [? 121.273003]? ret_from_fork+0x1f/0x40
>>
>> Signed-off-by: Ken Cox <jkc@redhat.com>
>> ---
>>  ?drivers/net/ethernet/intel/iavf/iavf_main.c | 2 +-
>>  ?1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
>> b/drivers/net/ethernet/intel/iavf/iavf_main.c
>> index 6c2afbc8acbcd..63eec7edbf60a 100644
>> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
>> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
>> @@ -3940,7 +3940,6 @@ static void iavf_remove(struct pci_dev *pdev)
>>  ????????set_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section);
>>  ????????cancel_delayed_work_sync(&adapter->init_task);
>>  ????????cancel_work_sync(&adapter->reset_task);
>> -???????cancel_delayed_work_sync(&adapter->client_task);
>>  ????????if (adapter->netdev_registered) {
>>  ????????????????unregister_netdev(netdev);
>>  ????????????????adapter->netdev_registered = false;
>> @@ -3974,6 +3973,7 @@ static void iavf_remove(struct pci_dev *pdev)
>>  ????????iavf_free_q_vectors(adapter);
>>   
>>  ????????cancel_delayed_work_sync(&adapter->watchdog_task);
>> +???????cancel_delayed_work_sync(&adapter->client_task);
>>   
>>  ????????cancel_work_sync(&adapter->adminq_task);
>>   
> 
> Hi Ken,
> 
> What tree is this patch based on? This doesn't apply to either of the
> IWL trees or the netdev trees.
Sorry, I was in the wrong branch when I generated these patches.  Please 
disregard.  I will re-evaluate and resend if necessary.

> 
> The ordering looks correct on the kernel tree with watchdog_task being
> cancelled before the client_task [1]. However, we do have an extra
> 'cancel_delayed_work_sync(&adapter->watchdog_task)'. I'll get a patch
> together to remove the extra one.
> 
> Thanks,
> Tony
> 
> 
> [1] https://elixir.bootlin.com/linux/v5.16-
> rc5/source/drivers/net/ethernet/intel/iavf/iavf_main.c#3979
>