RE: [PATCH v5 net-next 4/9] net: dsa: netc: tie setup resources and reset to DSA setup/teardown

quoted

+static int netc_switch_reset(struct netc_switch *priv)
+{
+	if (priv->in_probe)
+		return pci_reset_function_locked(priv->pdev);

[Severity: High]
Can a concurrent thread execute this path without holding the PCI device
lock?

In a multi-switch DSA tree, priv->in_probe is set to true during
netc_switch_probe(). The thread then calls dsa_register_switch(), which
internally acquires and releases dsa2_mutex. If the DSA tree is incomplete,
it returns, and the thread can be preempted before executing
priv->in_probe = false.

Concurrently, a second thread could probe the second switch, complete the
tree, acquire dsa2_mutex, and call dsa_tree_setup_switches(). This invokes
netc_setup() for the first switch. Observing priv->in_probe == true, the
second thread would call pci_reset_function_locked() on the first switch's
PCI device, despite not holding its device lock.

Yes, there is a potential issue, I will fix it

quoted

+
+	return pci_reset_function(priv->pdev);

[Severity: High]
Does this introduce an AB-BA deadlock between the PCI device lock and
dsa2_mutex?

When a multi-switch DSA tree is set up, netc_setup() can be called for an
already-probed switch. In this case, priv->in_probe is false, and
netc_switch_reset() calls pci_reset_function(), which acquires the PCI
device lock. However, netc_setup() is invoked by the DSA core with the
global dsa2_mutex held.

Conversely, when a switch is unbound (e.g. via sysfs), the driver core first
acquires the switch's device lock and then calls the driver's remove
callback. The netc_switch_remove() function calls dsa_unregister_switch(),
which attempts to acquire dsa2_mutex:

net/dsa/dsa.c:dsa_unregister_switch() {
    ...
    mutex_lock(&dsa2_mutex);
    dsa_switch_remove(ds);
    mutex_unlock(&dsa2_mutex);
    ...
}

If Thread 1 is unbinding Switch A (holding the device lock for A and waiting
for dsa2_mutex) while Thread 2 is probing Switch B (holding dsa2_mutex and
calling pci_reset_function(A) inside netc_setup(A)), the system could
deadlock.

--
pw-bot: cr