Commit 0579fafd authored by Slawomir Laba's avatar Slawomir Laba Committed by Tony Nguyen
Browse files

iavf: Fix locking for VIRTCHNL_OP_GET_OFFLOAD_VLAN_V2_CAPS 



iavf_virtchnl_completion is called under crit_lock but when
the code for VIRTCHNL_OP_GET_OFFLOAD_VLAN_V2_CAPS is called,
this lock is released in order to obtain rtnl_lock to avoid
ABBA deadlock with unregister_netdev.

Along with the new way iavf_remove behaves, there exist
many risks related to the lock release and attmepts to regrab
it. The driver faces crashes related to races between
unregister_netdev and netdev_update_features. Yet another
risk is that the driver could already obtain the crit_lock
in order to destroy it and iavf_virtchnl_completion could
crash or block forever.

Make iavf_virtchnl_completion never relock crit_lock in it's
call paths.

Extract rtnl_lock locking logic to the driver for
unregister_netdev in order to set the netdev_registered flag
inside the lock.

Introduce a new flag that will inform adminq_task to perform
the code from VIRTCHNL_OP_GET_OFFLOAD_VLAN_V2_CAPS right after
it finishes processing messages. Guard this code with remove
flags so it's never called when the driver is in remove state.

Fixes: 5951a2b9 ("iavf: Fix VLAN feature flags after VFR")
Signed-off-by: default avatarSlawomir Laba <slawomirx.laba@intel.com>
Signed-off-by: default avatarPhani Burra <phani.r.burra@intel.com>
Signed-off-by: default avatarJacob Keller <jacob.e.keller@intel.com>
Signed-off-by: default avatarMateusz Palczewski <mateusz.palczewski@intel.com>
Tested-by: default avatarKonrad Jankowski <konrad0.jankowski@intel.com>
Signed-off-by: default avatarTony Nguyen <anthony.l.nguyen@intel.com>
parent 3ccd54ef

drivers/net/ethernet/intel/iavf/iavf.h

+1 −0
Original line number Diff line number Diff line
@@ -287,6 +287,7 @@ struct iavf_adapter { 
#define IAVF_FLAG_LEGACY_RX			BIT(15)

#define IAVF_FLAG_REINIT_ITR_NEEDED		BIT(16)

#define IAVF_FLAG_QUEUES_DISABLED		BIT(17)

#define IAVF_FLAG_SETUP_NETDEV_FEATURES		BIT(18)

/* duplicates for common code */

#define IAVF_FLAG_DCB_ENABLED			0

	/* flags for admin queue service task */

drivers/net/ethernet/intel/iavf/iavf_main.c

+21 −1
Original line number Diff line number Diff line
@@ -2879,6 +2879,24 @@ static void iavf_adminq_task(struct work_struct *work) 
	} while (pending);

	mutex_unlock(&adapter->crit_lock);



	if ((adapter->flags & IAVF_FLAG_SETUP_NETDEV_FEATURES)) {

		if (adapter->netdev_registered ||

		    !test_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section)) {

			struct net_device *netdev = adapter->netdev;



			rtnl_lock();

			netdev_update_features(netdev);

			rtnl_unlock();

			/* Request VLAN offload settings */

			if (VLAN_V2_ALLOWED(adapter))

				iavf_set_vlan_offload_features

					(adapter, 0, netdev->features);



			iavf_set_queue_vlan_tag_loc(adapter);

		}



		adapter->flags &= ~IAVF_FLAG_SETUP_NETDEV_FEATURES;

	}

	if ((adapter->flags &

	     (IAVF_FLAG_RESET_PENDING | IAVF_FLAG_RESET_NEEDED)) ||

	    adapter->state == __IAVF_RESETTING)
@@ -4606,8 +4624,10 @@ static void iavf_remove(struct pci_dev *pdev) 
	cancel_delayed_work_sync(&adapter->watchdog_task);



	if (adapter->netdev_registered) {

		unregister_netdev(netdev);

		rtnl_lock();

		unregister_netdevice(netdev);

		adapter->netdev_registered = false;

		rtnl_unlock();

	}

	if (CLIENT_ALLOWED(adapter)) {

		err = iavf_lan_del_device(adapter);

drivers/net/ethernet/intel/iavf/iavf_virtchnl.c

+1 −23
Original line number Diff line number Diff line
@@ -2146,29 +2146,7 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter, 
				     sizeof(adapter->vlan_v2_caps)));



		iavf_process_config(adapter);



		/* unlock crit_lock before acquiring rtnl_lock as other

		 * processes holding rtnl_lock could be waiting for the same

		 * crit_lock

		 */

		mutex_unlock(&adapter->crit_lock);

		/* VLAN capabilities can change during VFR, so make sure to

		 * update the netdev features with the new capabilities

		 */

		rtnl_lock();

		netdev_update_features(netdev);

		rtnl_unlock();

		if (iavf_lock_timeout(&adapter->crit_lock, 10000))

			dev_warn(&adapter->pdev->dev, "failed to acquire crit_lock in %s\n",

				 __FUNCTION__);



		/* Request VLAN offload settings */

		if (VLAN_V2_ALLOWED(adapter))

			iavf_set_vlan_offload_features(adapter, 0,

						       netdev->features);



		iavf_set_queue_vlan_tag_loc(adapter);



		adapter->flags |= IAVF_FLAG_SETUP_NETDEV_FEATURES;

		}

		break;

	case VIRTCHNL_OP_ENABLE_QUEUES: