[PATCH net v1] ncsi: Fix use-after-free in the VLAN id list removal

[PATCH net v1] ncsi: Fix use-after-free in the VLAN id list removal

[Binbin Deng]

ncsi_vlan_rx_kill_vid() removes an entry from the NCSI VLAN list with:

	list_for_each_entry_safe(vlan, tmp, &ndp->vlan_vids, list)
		if (vlan->vid == vid) {
			list_del_rcu(&vlan->list);
			found = true;
			kfree(vlan);
		}

The list is published with list_add_rcu(), so readers traverse it
under rcu_read_lock().  The writer holds only the RTNL, which does not
exclude those readers.  list_for_each_entry_safe() protects the writer
iteration itself, not concurrent readers.

The reader is set_one_vid(), invoked from the NCSI channel
configuration state machine (ndp->work) while sending the "Set VLAN
Filter" command:

	rcu_read_lock();
	list_for_each_entry_rcu(vlan, &ndp->vlan_vids, list) {
		vid = vlan->vid;
		...
	}
	rcu_read_unlock();

An RCU read-side critical section only protects objects released via
call_rcu()/kfree_rcu(); it does not protect against the immediate
kfree() above.  If the state machine work has already obtained the
list entry pointer and is then scheduled out while another CPU removes
the VLAN id, the resumed iteration reads the freed node and follows
the ->next pointer of reused memory.

This is reachable on BMC systems using NCSI when a VLAN id is deleted
(ip link / rtnetlink) concurrently with an NCSI configuration cycle.

Fix this by adding an rcu_head to struct vlan_vid and releasing the
node with kfree_rcu().

Fixes: 21acf63013ed ("net/ncsi: Configure VLAN tag filter")
Signed-off-by: Binbin Deng <18983559317@163.com>
---
 net/ncsi/internal.h    | 1 +
 net/ncsi/ncsi-manage.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index adee6dcabdc3..2c9d1f22c16a 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -312,6 +312,7 @@ struct vlan_vid {
 	struct list_head list;
 	__be16 proto;
 	u16 vid;
+	struct rcu_head rcu_head;
 };
 
 struct ncsi_dev_priv {
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 54d0df0a9efe..1d63958c4429 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -1737,7 +1737,7 @@ int ncsi_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, u16 vid)
 			netdev_dbg(dev, "NCSI: vid %u found, removing\n", vid);
 			list_del_rcu(&vlan->list);
 			found = true;
-			kfree(vlan);
+			kfree_rcu(vlan, rcu_head);
 		}
 
 	if (!found) {
-- 
2.43.0

Re: [PATCH net v1] ncsi: Fix use-after-free in the VLAN id list removal

[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:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

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.