[PATCH v1] qlcnic: Protect loopback filter hashes

[PATCH v1] qlcnic: Protect loopback filter hashes

[Binbin Deng]

QLCNIC tracks learned MAC filters in two hash tables, but the existing
mac_learn_lock and rx_mac_learn_lock protect only some insertion and
deletion operations.  Filter aging and interface teardown traverse,
unhash, and free nodes while the TX and loopback RX paths traverse or
update the same buckets without holding the corresponding lock.

The race is reachable when filter aging or interface down runs while
packets create or refresh learned filters on another CPU.  A transmit or
loopback lookup can follow a node that the aging task has just freed,
and concurrent modification can corrupt a bucket.  The result can be a
use-after-free detected by KASAN, corrupted filter accounting, or a panic.

Extend the two existing locks over each complete hash-table operation:
aging and teardown hold the lock while traversing, deleting from the
firmware, unhashing, and freeing; the transmit and loopback receive paths
hold it while checking the count, looking up, updating timestamps, and
inserting a new node.  The existing firmware-command path is already called
under these spinlocks, so no new lock class or lock ordering is introduced.

Found during a static-analysis-assisted audit: candidate sites were
surfaced by a Coccinelle rule, and the reader/writer paths were
confirmed by LLM-assisted (GLM-5.3) code inspection.  The issue was
not observed at runtime; it is identified by code inspection only.

Fixes: b5e5492c0d49 ("qlcnic: support mac learning")
Fixes: 53643a75b147 ("qlcnic: fix ping resumption to a VM after a live migration")
Assisted-by: GLM-5.3 coccinelle
Signed-off-by: Binbin Deng <18983559317@163.com>
---
 .../net/ethernet/qlogic/qlcnic/qlcnic_hw.c    | 12 +++++------
 .../net/ethernet/qlogic/qlcnic/qlcnic_io.c    | 21 +++++++++++++------
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c
index e58176badc1f..c7309ecf6023 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c
@@ -635,6 +635,7 @@ void qlcnic_prune_lb_filters(struct qlcnic_adapter *adapter)
 	unsigned long expires;
 	u8 cmd;
 
+	spin_lock_bh(&adapter->mac_learn_lock);
 	for (i = 0; i < adapter->fhash.fbucket_size; i++) {
 		head = &(adapter->fhash.fhead[i]);
 		hlist_for_each_entry_safe(tmp_fil, n, head, fnode) {
@@ -646,14 +647,14 @@ void qlcnic_prune_lb_filters(struct qlcnic_adapter *adapter)
 							  tmp_fil->faddr,
 							  tmp_fil->vlan_id,
 							  cmd);
-				spin_lock_bh(&adapter->mac_learn_lock);
 				adapter->fhash.fnum--;
 				hlist_del(&tmp_fil->fnode);
-				spin_unlock_bh(&adapter->mac_learn_lock);
 				kfree(tmp_fil);
 			}
 		}
 	}
+	spin_unlock_bh(&adapter->mac_learn_lock);
+	spin_lock_bh(&adapter->rx_mac_learn_lock);
 	for (i = 0; i < adapter->rx_fhash.fbucket_size; i++) {
 		head = &(adapter->rx_fhash.fhead[i]);
 
@@ -661,14 +662,13 @@ void qlcnic_prune_lb_filters(struct qlcnic_adapter *adapter)
 		{
 			expires = tmp_fil->ftime + QLCNIC_FILTER_AGE * HZ;
 			if (time_before(expires, jiffies)) {
-				spin_lock_bh(&adapter->rx_mac_learn_lock);
 				adapter->rx_fhash.fnum--;
 				hlist_del(&tmp_fil->fnode);
-				spin_unlock_bh(&adapter->rx_mac_learn_lock);
 				kfree(tmp_fil);
 			}
 		}
 	}
+	spin_unlock_bh(&adapter->rx_mac_learn_lock);
 }
 
 void qlcnic_delete_lb_filters(struct qlcnic_adapter *adapter)
@@ -679,6 +679,7 @@ void qlcnic_delete_lb_filters(struct qlcnic_adapter *adapter)
 	int i;
 	u8 cmd;
 
+	spin_lock_bh(&adapter->mac_learn_lock);
 	for (i = 0; i < adapter->fhash.fbucket_size; i++) {
 		head = &(adapter->fhash.fhead[i]);
 		hlist_for_each_entry_safe(tmp_fil, n, head, fnode) {
@@ -688,13 +689,12 @@ void qlcnic_delete_lb_filters(struct qlcnic_adapter *adapter)
 						  tmp_fil->faddr,
 						  tmp_fil->vlan_id,
 						  cmd);
-			spin_lock_bh(&adapter->mac_learn_lock);
 			adapter->fhash.fnum--;
 			hlist_del(&tmp_fil->fnode);
-			spin_unlock_bh(&adapter->mac_learn_lock);
 			kfree(tmp_fil);
 		}
 	}
+	spin_unlock_bh(&adapter->mac_learn_lock);
 }
 
 static int qlcnic_set_fw_loopback(struct qlcnic_adapter *adapter, u8 flag)
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
index 761ef3bc8193..2aefb6e9a2eb 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
@@ -207,27 +207,33 @@ static void qlcnic_add_lb_filter(struct qlcnic_adapter *adapter,
 		 (adapter->fhash.fbucket_size - 1);
 
 	if (loopback_pkt) {
-		if (adapter->rx_fhash.fnum >= adapter->rx_fhash.fmax)
-			return;
 
 		head = &(adapter->rx_fhash.fhead[hindex]);
 
+		spin_lock(&adapter->rx_mac_learn_lock);
+		if (adapter->rx_fhash.fnum >= adapter->rx_fhash.fmax) {
+			spin_unlock(&adapter->rx_mac_learn_lock);
+			return;
+		}
+
 		tmp_fil = qlcnic_find_mac_filter(head, &src_addr, vlan_id);
 		if (tmp_fil) {
 			time = tmp_fil->ftime;
 			if (time_after(jiffies, QLCNIC_READD_AGE * HZ + time))
 				tmp_fil->ftime = jiffies;
+			spin_unlock(&adapter->rx_mac_learn_lock);
 			return;
 		}
 
 		fil = kzalloc_obj(struct qlcnic_filter, GFP_ATOMIC);
-		if (!fil)
+		if (!fil) {
+			spin_unlock(&adapter->rx_mac_learn_lock);
 			return;
+		}
 
 		fil->ftime = jiffies;
 		memcpy(fil->faddr, &src_addr, ETH_ALEN);
 		fil->vlan_id = vlan_id;
-		spin_lock(&adapter->rx_mac_learn_lock);
 		hlist_add_head(&(fil->fnode), head);
 		adapter->rx_fhash.fnum++;
 		spin_unlock(&adapter->rx_mac_learn_lock);
@@ -330,6 +336,7 @@ static void qlcnic_send_filter(struct qlcnic_adapter *adapter,
 	hindex = hval & (adapter->fhash.fbucket_size - 1);
 	head = &(adapter->fhash.fhead[hindex]);
 
+	spin_lock(&adapter->mac_learn_lock);
 	hlist_for_each_entry_safe(tmp_fil, n, head, fnode) {
 		if (ether_addr_equal(tmp_fil->faddr, (u8 *)&src_addr) &&
 		    tmp_fil->vlan_id == vlan_id) {
@@ -337,6 +344,7 @@ static void qlcnic_send_filter(struct qlcnic_adapter *adapter,
 				qlcnic_change_filter(adapter, &src_addr,
 						     vlan_id, tx_ring);
 			tmp_fil->ftime = jiffies;
+			spin_unlock(&adapter->mac_learn_lock);
 			return;
 		}
 	}
@@ -347,14 +355,15 @@ static void qlcnic_send_filter(struct qlcnic_adapter *adapter,
 	}
 
 	fil = kzalloc_obj(struct qlcnic_filter, GFP_ATOMIC);
-	if (!fil)
+	if (!fil) {
+		spin_unlock(&adapter->mac_learn_lock);
 		return;
+	}
 
 	qlcnic_change_filter(adapter, &src_addr, vlan_id, tx_ring);
 	fil->ftime = jiffies;
 	fil->vlan_id = vlan_id;
 	memcpy(fil->faddr, &src_addr, ETH_ALEN);
-	spin_lock(&adapter->mac_learn_lock);
 	hlist_add_head(&(fil->fnode), head);
 	adapter->fhash.fnum++;
 	spin_unlock(&adapter->mac_learn_lock);
-- 
2.43.0

Re: [PATCH v1] qlcnic: Protect loopback filter hashes

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re:Re: [PATCH v1] qlcnic: Protect loopback filter hashes

[Binbin Deng]

Due to a lack of specific hardware, this change has not been tested on
actual hardware. But in my opinion, this fix is necessary.