[PATCH v6] net/ncsi: Fix Use-After-Free in NCSI teardown using synchronize_rcu

[PATCH v6] net/ncsi: Fix Use-After-Free in NCSI teardown using synchronize_rcu

[Wong Boon Jhee]

ncsi_remove_package() and ncsi_remove_channel() remove objects from
RCU-protected lists and free them immediately using kfree(). Concurrent
readers (such as Netlink dump handlers) traversing these lists may
access freed memory, resulting in a slab-use-after-free.

Instead of converting all frees to kfree_rcu() or adding complex
reference counting, this fix uses synchronize_rcu()
in the teardown path (ncsi_unregister_dev). By stopping asynchronous
producers and waiting for all RCU readers to finish before destroying
the device tree, we ensure safe synchronous reclamation.

This approach is compact, avoids overhead on the fast path, and
resolves the race condition reported by KASAN.

Fixes: 2d283bdd079c ("net/ncsi: Resource management")
Signed-off-by: Wong Boon Jhee <wongboonjhee52@gmail.com>
---
v5 -> v6:
- Added RCU read-side protection to all NCSI netlink handlers and fixed
  every error-path unlock.
- Moved synchronize_rcu() before package/channel iteration during teardown.
- Stopped channel monitors before draining request timers and released all
  outstanding request command/response SKBs.
- Removed the unnecessary rcu_barrier() and drained ndp->vlan_vids.
- Documented the updated teardown ordering.
 net/ncsi/ncsi-manage.c  | 34 +++++++++++++++++----
 net/ncsi/ncsi-netlink.c | 79 ++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 91 insertions(+), 22 deletions(-)

diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 54d0df0a9efe..00941c630270 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -1956,19 +1956,43 @@ void ncsi_unregister_dev(struct ncsi_dev *nd)
 {
 	struct ncsi_dev_priv *ndp = TO_NCSI_DEV_PRIV(nd);
 	struct ncsi_package *np, *tmp;
+	struct ncsi_channel *nc;
+	struct vlan_vid *vlan, *vlan_tmp;
 	unsigned long flags;
-
-	dev_remove_pack(&ndp->ptype);
-
-	list_for_each_entry_safe(np, tmp, &ndp->packages, node)
-		ncsi_remove_package(np);
+	int i;
 
 	spin_lock_irqsave(&ncsi_dev_lock, flags);
 	list_del_rcu(&ndp->node);
 	spin_unlock_irqrestore(&ncsi_dev_lock, flags);
 
+	dev_remove_pack(&ndp->ptype);
 	disable_work_sync(&ndp->work);
 
+	/* Wait for readers that found this device before it was unlinked. */
+	synchronize_rcu();
+
+	/* Stop all request producers before draining request state. */
+	list_for_each_entry(np, &ndp->packages, node) {
+		list_for_each_entry(nc, &np->channels, node)
+			ncsi_stop_channel_monitor(nc);
+	}
+
+	for (i = 0; i < ARRAY_SIZE(ndp->requests); i++) {
+		struct ncsi_request *nr = &ndp->requests[i];
+
+		timer_delete_sync(&nr->timer);
+		if (nr->used)
+			ncsi_free_request(nr);
+	}
+
+	list_for_each_entry_safe(vlan, vlan_tmp, &ndp->vlan_vids, list) {
+		list_del(&vlan->list);
+		kfree(vlan);
+	}
+
+	list_for_each_entry_safe(np, tmp, &ndp->packages, node)
+		ncsi_remove_package(np);
+
 	kfree(ndp);
 }
 EXPORT_SYMBOL_GPL(ncsi_unregister_dev);
diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c
index 8cc538358f6a..f8ea2ea73fb0 100644
--- a/net/ncsi/ncsi-netlink.c
+++ b/net/ncsi/ncsi-netlink.c
@@ -169,19 +169,24 @@ static int ncsi_pkg_info_nl(struct sk_buff *msg, struct genl_info *info)
 	if (!info->attrs[NCSI_ATTR_PACKAGE_ID])
 		return -EINVAL;
 
-	ndp = ndp_from_ifindex(genl_info_net(info),
-			       nla_get_u32(info->attrs[NCSI_ATTR_IFINDEX]));
-	if (!ndp)
-		return -ENODEV;
-
 	skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!skb)
 		return -ENOMEM;
 
+	rcu_read_lock();
+	ndp = ndp_from_ifindex(genl_info_net(info),
+			       nla_get_u32(info->attrs[NCSI_ATTR_IFINDEX]));
+	if (!ndp) {
+		rcu_read_unlock();
+		kfree_skb(skb);
+		return -ENODEV;
+	}
+
 	hdr = genlmsg_put(skb, info->snd_portid, info->snd_seq,
 			  &ncsi_genl_family, 0, NCSI_CMD_PKG_INFO);
 	if (!hdr) {
 		kfree_skb(skb);
+		rcu_read_unlock();
 		return -EMSGSIZE;
 	}
 
@@ -190,6 +195,7 @@ static int ncsi_pkg_info_nl(struct sk_buff *msg, struct genl_info *info)
 	attr = nla_nest_start_noflag(skb, NCSI_ATTR_PACKAGE_LIST);
 	if (!attr) {
 		kfree_skb(skb);
+		rcu_read_unlock();
 		return -EMSGSIZE;
 	}
 	rc = ncsi_write_package_info(skb, ndp, package_id);
@@ -202,10 +208,12 @@ static int ncsi_pkg_info_nl(struct sk_buff *msg, struct genl_info *info)
 	nla_nest_end(skb, attr);
 
 	genlmsg_end(skb, hdr);
+	rcu_read_unlock();
 	return genlmsg_reply(skb, info);
 
 err:
 	kfree_skb(skb);
+	rcu_read_unlock();
 	return rc;
 }
 
@@ -228,11 +236,14 @@ static int ncsi_pkg_info_all_nl(struct sk_buff *skb,
 	if (!attrs[NCSI_ATTR_IFINDEX])
 		return -EINVAL;
 
+	rcu_read_lock();
 	ndp = ndp_from_ifindex(get_net(sock_net(skb->sk)),
 			       nla_get_u32(attrs[NCSI_ATTR_IFINDEX]));
 
-	if (!ndp)
+	if (!ndp) {
+		rcu_read_unlock();
 		return -ENODEV;
+	}
 
 	package_id = cb->args[0];
 	package = NULL;
@@ -240,20 +251,23 @@ static int ncsi_pkg_info_all_nl(struct sk_buff *skb,
 		if (np->id == package_id)
 			package = np;
 
-	if (!package)
+	if (!package) {
+		rcu_read_unlock();
 		return 0; /* done */
+	}
 
 	hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
 			  &ncsi_genl_family, NLM_F_MULTI,  NCSI_CMD_PKG_INFO);
 	if (!hdr) {
-		rc = -EMSGSIZE;
-		goto err;
+		rcu_read_unlock();
+		return -EMSGSIZE;
 	}
 
 	attr = nla_nest_start_noflag(skb, NCSI_ATTR_PACKAGE_LIST);
 	if (!attr) {
-		rc = -EMSGSIZE;
-		goto err;
+		genlmsg_cancel(skb, hdr);
+		rcu_read_unlock();
+		return -EMSGSIZE;
 	}
 	rc = ncsi_write_package_info(skb, ndp, package->id);
 	if (rc) {
@@ -266,9 +280,12 @@ static int ncsi_pkg_info_all_nl(struct sk_buff *skb,
 
 	cb->args[0] = package_id + 1;
 
-	return skb->len;
+	rc = skb->len;
+	rcu_read_unlock();
+	return rc;
 err:
 	genlmsg_cancel(skb, hdr);
+	rcu_read_unlock();
 	return rc;
 }
 
@@ -289,10 +306,13 @@ static int ncsi_set_interface_nl(struct sk_buff *msg, struct genl_info *info)
 	if (!info->attrs[NCSI_ATTR_PACKAGE_ID])
 		return -EINVAL;
 
+	rcu_read_lock();
 	ndp = ndp_from_ifindex(get_net(sock_net(msg->sk)),
 			       nla_get_u32(info->attrs[NCSI_ATTR_IFINDEX]));
-	if (!ndp)
+	if (!ndp) {
+		rcu_read_unlock();
 		return -ENODEV;
+	}
 
 	package_id = nla_get_u32(info->attrs[NCSI_ATTR_PACKAGE_ID]);
 	package = NULL;
@@ -302,6 +322,7 @@ static int ncsi_set_interface_nl(struct sk_buff *msg, struct genl_info *info)
 			package = np;
 	if (!package) {
 		/* The user has set a package that does not exist */
+		rcu_read_unlock();
 		return -ERANGE;
 	}
 
@@ -317,6 +338,7 @@ static int ncsi_set_interface_nl(struct sk_buff *msg, struct genl_info *info)
 			netdev_info(ndp->ndev.dev,
 				    "NCSI: Channel %u does not exist!\n",
 				    channel_id);
+			rcu_read_unlock();
 			return -ERANGE;
 		}
 	}
@@ -350,6 +372,7 @@ static int ncsi_set_interface_nl(struct sk_buff *msg, struct genl_info *info)
 	if (!(ndp->flags & NCSI_DEV_RESET))
 		ncsi_reset_dev(&ndp->ndev);
 
+	rcu_read_unlock();
 	return 0;
 }
 
@@ -365,10 +388,13 @@ static int ncsi_clear_interface_nl(struct sk_buff *msg, struct genl_info *info)
 	if (!info->attrs[NCSI_ATTR_IFINDEX])
 		return -EINVAL;
 
+	rcu_read_lock();
 	ndp = ndp_from_ifindex(get_net(sock_net(msg->sk)),
 			       nla_get_u32(info->attrs[NCSI_ATTR_IFINDEX]));
-	if (!ndp)
+	if (!ndp) {
+		rcu_read_unlock();
 		return -ENODEV;
+	}
 
 	/* Reset any whitelists and disable multi mode */
 	spin_lock_irqsave(&ndp->lock, flags);
@@ -389,6 +415,7 @@ static int ncsi_clear_interface_nl(struct sk_buff *msg, struct genl_info *info)
 	if (!(ndp->flags & NCSI_DEV_RESET))
 		ncsi_reset_dev(&ndp->ndev);
 
+	rcu_read_unlock();
 	return 0;
 }
 
@@ -398,6 +425,7 @@ static int ncsi_send_cmd_nl(struct sk_buff *msg, struct genl_info *info)
 	struct ncsi_pkt_hdr *hdr;
 	struct ncsi_cmd_arg nca;
 	unsigned char *data;
+	bool rcu_locked = false;
 	u32 package_id;
 	u32 channel_id;
 	int len, ret;
@@ -427,10 +455,14 @@ static int ncsi_send_cmd_nl(struct sk_buff *msg, struct genl_info *info)
 		goto out;
 	}
 
+	rcu_read_lock();
+	rcu_locked = true;
 	ndp = ndp_from_ifindex(get_net(sock_net(msg->sk)),
 			       nla_get_u32(info->attrs[NCSI_ATTR_IFINDEX]));
 	if (!ndp) {
 		ret = -ENODEV;
+		rcu_read_unlock();
+		rcu_locked = false;
 		goto out;
 	}
 
@@ -480,6 +512,8 @@ static int ncsi_send_cmd_nl(struct sk_buff *msg, struct genl_info *info)
 				      ret);
 	}
 out:
+	if (rcu_locked)
+		rcu_read_unlock();
 	return ret;
 }
 
@@ -608,10 +642,13 @@ static int ncsi_set_package_mask_nl(struct sk_buff *msg,
 	if (!info->attrs[NCSI_ATTR_PACKAGE_MASK])
 		return -EINVAL;
 
+	rcu_read_lock();
 	ndp = ndp_from_ifindex(get_net(sock_net(msg->sk)),
 			       nla_get_u32(info->attrs[NCSI_ATTR_IFINDEX]));
-	if (!ndp)
+	if (!ndp) {
+		rcu_read_unlock();
 		return -ENODEV;
+	}
 
 	spin_lock_irqsave(&ndp->lock, flags);
 	if (nla_get_flag(info->attrs[NCSI_ATTR_MULTI_FLAG])) {
@@ -639,6 +676,7 @@ static int ncsi_set_package_mask_nl(struct sk_buff *msg,
 			ncsi_reset_dev(&ndp->ndev);
 	}
 
+	rcu_read_unlock();
 	return rc;
 }
 
@@ -663,10 +701,13 @@ static int ncsi_set_channel_mask_nl(struct sk_buff *msg,
 	if (!info->attrs[NCSI_ATTR_CHANNEL_MASK])
 		return -EINVAL;
 
+	rcu_read_lock();
 	ndp = ndp_from_ifindex(get_net(sock_net(msg->sk)),
 			       nla_get_u32(info->attrs[NCSI_ATTR_IFINDEX]));
-	if (!ndp)
+	if (!ndp) {
+		rcu_read_unlock();
 		return -ENODEV;
+	}
 
 	package_id = nla_get_u32(info->attrs[NCSI_ATTR_PACKAGE_ID]);
 	package = NULL;
@@ -675,8 +716,10 @@ static int ncsi_set_channel_mask_nl(struct sk_buff *msg,
 			package = np;
 			break;
 		}
-	if (!package)
+	if (!package) {
+		rcu_read_unlock();
 		return -ERANGE;
+	}
 
 	spin_lock_irqsave(&package->lock, flags);
 
@@ -690,6 +733,7 @@ static int ncsi_set_channel_mask_nl(struct sk_buff *msg,
 			}
 		if (!channel) {
 			spin_unlock_irqrestore(&package->lock, flags);
+			rcu_read_unlock();
 			return -ERANGE;
 		}
 		netdev_dbg(ndp->ndev.dev,
@@ -721,6 +765,7 @@ static int ncsi_set_channel_mask_nl(struct sk_buff *msg,
 	if (!(ndp->flags & NCSI_DEV_RESET))
 		ncsi_reset_dev(&ndp->ndev);
 
+	rcu_read_unlock();
 	return 0;
 }
 

Re: [PATCH v6] net/ncsi: Fix Use-After-Free in NCSI teardown using synchronize_rcu

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 2 (1) · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix's reclamation model (list_del_rcu(&ndp->node) ->
  synchronize_rcu() -> kfree everything) is only correct if every reader…
- [High] The new 'Stop all request producers' loop in
  ncsi_unregister_dev() does not guarantee that an executing channel-…
- [Medium] The newly added vlan_vids drain in ncsi_unregister_dev() uses
  plain list_del() followed by immediate kfree() on ndp->vlan_vids, a…
- [Low] Unrelated fixes bundled under one Fixes: tag.

Pre-existing issues:
- [High] Peer call-site inconsistency / net namespace reference leak
  around ndp_from_ifindex().

--

net/ncsi: Fix Use-After-Free in NCSI teardown using synchronize_rcu

This patch reorders ncsi_unregister_dev() so the device is unlinked from
ncsi_dev_list first, followed by synchronize_rcu(), stopping channel
monitors, draining ndp->requests[] and ndp->vlan_vids, and only then
freeing packages, channels and ndp.  It also wraps the NCSI netlink
handlers in rcu_read_lock()/rcu_read_unlock() around the ndp lookup and
use.

A few questions below about the new teardown ordering and about the
netlink handlers.

> diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> index 54d0df0a9efe6..00941c6302700 100644
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -1956,19 +1956,43 @@ void ncsi_unregister_dev(struct ncsi_dev *nd)
>  {
>  	struct ncsi_dev_priv *ndp = TO_NCSI_DEV_PRIV(nd);
>  	struct ncsi_package *np, *tmp;
> +	struct ncsi_channel *nc;
> +	struct vlan_vid *vlan, *vlan_tmp;
>  	unsigned long flags;
> -
> -	dev_remove_pack(&ndp->ptype);
> -
> -	list_for_each_entry_safe(np, tmp, &ndp->packages, node)
> -		ncsi_remove_package(np);
> +	int i;
>  
>  	spin_lock_irqsave(&ncsi_dev_lock, flags);
>  	list_del_rcu(&ndp->node);
>  	spin_unlock_irqrestore(&ncsi_dev_lock, flags);
>  
> +	dev_remove_pack(&ndp->ptype);
>  	disable_work_sync(&ndp->work);
>  
> +	/* Wait for readers that found this device before it was unlinked. */
> +	synchronize_rcu();

[Severity: High]
Does this synchronize_rcu() actually cover every reader that can obtain
ndp?  The reclamation model here (list_del_rcu(&ndp->node) then
synchronize_rcu() then kfree of everything) only holds if all readers walk
ncsi_dev_list inside an RCU read-side section.

ncsi_vlan_rx_add_vid() and ncsi_vlan_rx_kill_vid() look like they do the
lookup with no rcu_read_lock() at all:

net/ncsi/ncsi-manage.c:ncsi_vlan_rx_add_vid() {
	nd = ncsi_find_dev(dev);
	...
	ndp = TO_NCSI_DEV_PRIV(nd);

	/* Add the VLAN id to our internal list */
	list_for_each_entry_rcu(vlan, &ndp->vlan_vids, list) {
	...
	ncsi_kick_channels(ndp);
	...
}

and ncsi_find_dev() traverses the list with the RCU helper:

net/ncsi/ncsi-manage.c:ncsi_find_dev() {
	NCSI_FOR_EACH_DEV(ndp) {      /* list_for_each_entry_rcu() */
		if (ndp->ndev.dev == dev)
			return &ndp->ndev;
	}
}

So can this sequence still hit freed memory?

CPU0: ncsi_vlan_rx_add_vid()
	nd = ncsi_find_dev(dev);        /* returns ndp, no RCU section */
	<preempted>

CPU1: ncsi_unregister_dev()
	list_del_rcu(&ndp->node);
	synchronize_rcu();              /* CPU0 is not an RCU reader */
	... frees requests, vlan entries, channels, packages ...
	kfree(ndp);

CPU0: resumes and dereferences ndp (ndp->vlan_vids, list_add_rcu(),
      ncsi_kick_channels(ndp), ncsi_process_next_channel(ndp))

ftgmac100_remove() calls ncsi_unregister_dev() before unregister_netdev(),
and the ftgmac100_open() error path unregisters NCSI while the netdev stays
registered, so the ndo_vlan_rx_add_vid/kill_vid callbacks are still
reachable from user space during teardown.  Would those two callbacks also
need rcu_read_lock() (they additionally trip the CONFIG_PROVE_RCU
"RCU-list traversed in non-reader section" assertion today)?

> +
> +	/* Stop all request producers before draining request state. */
> +	list_for_each_entry(np, &ndp->packages, node) {
> +		list_for_each_entry(nc, &np->channels, node)
> +			ncsi_stop_channel_monitor(nc);
> +	}

[Severity: High]
Is this loop enough to guarantee the monitor timer callback has finished?
ncsi_stop_channel_monitor() returns early without any synchronization when
the flag is already clear:

net/ncsi/ncsi-manage.c:ncsi_stop_channel_monitor() {
	spin_lock_irqsave(&nc->lock, flags);
	if (!nc->monitor.enabled) {
		spin_unlock_irqrestore(&nc->lock, flags);
		return;
	}
	nc->monitor.enabled = false;
	spin_unlock_irqrestore(&nc->lock, flags);

	timer_delete_sync(&nc->monitor.timer);
}

but ncsi_channel_monitor() clears that same flag itself and then keeps
touching nc and ndp, both on the bad_state path and on the
"NCSI Channel %d timed out!" default path:

net/ncsi/ncsi-manage.c:ncsi_channel_monitor() {
	...
		spin_lock_irqsave(&nc->lock, flags);
		nc->monitor.enabled = false;
		nc->state = NCSI_CHANNEL_INVISIBLE;
		ncm->data[2] &= ~0x1;
		spin_unlock_irqrestore(&nc->lock, flags);

		spin_lock_irqsave(&ndp->lock, flags);
		nc->state = NCSI_CHANNEL_ACTIVE;
		list_add_tail_rcu(&nc->link, &ndp->channel_queue);
		spin_unlock_irqrestore(&ndp->lock, flags);
		ncsi_process_next_channel(ndp);
		return;
}

So if the callback is running on another CPU and has just dropped nc->lock
after clearing monitor.enabled, the new loop becomes a no-op and teardown
continues to ncsi_remove_package() -> ncsi_remove_channel() -> kfree(nc),
kfree(np), then kfree(ndp).  Can the callback then write into freed memory
via the list_add_tail_rcu() into ndp->channel_queue and the nc->state
store?  The earlier synchronize_rcu() does not help here, since the timer
callback is not an RCU reader of ncsi_dev_list.

Also, ncsi_remove_channel() already calls ncsi_stop_channel_monitor(nc),
so is the extra loop doing anything beyond duplicating that call?

> +
> +	for (i = 0; i < ARRAY_SIZE(ndp->requests); i++) {
> +		struct ncsi_request *nr = &ndp->requests[i];
> +
> +		timer_delete_sync(&nr->timer);
> +		if (nr->used)
> +			ncsi_free_request(nr);
> +	}
> +
> +	list_for_each_entry_safe(vlan, vlan_tmp, &ndp->vlan_vids, list) {
> +		list_del(&vlan->list);
> +		kfree(vlan);
> +	}

[Severity: Medium]
Is plain list_del() the right unlink here?  ndp->vlan_vids is published and
traversed with the RCU helpers elsewhere in the same file:

net/ncsi/ncsi-manage.c:ncsi_vlan_rx_add_vid() {
	list_for_each_entry_rcu(vlan, &ndp->vlan_vids, list) {
	...
	list_add_rcu(&vlan->list, &ndp->vlan_vids);
}

list_del() stores LIST_POISON1 into entry->next, so a concurrent
list_for_each_entry_rcu() walk would follow a poisoned pointer, and the
kfree() is immediate rather than deferred past a grace period.  Note the
synchronize_rcu() earlier in this function happens before these unlinks, so
it does not cover them.  Would list_del_rcu() plus a deferred free (or
performing this drain only after the readers are provably quiesced) be more
appropriate?

[Severity: Low]
This isn't a bug, but the vlan_vids drain and the ndp->requests[] drain
look like separate fixes from the RCU reclamation change, and neither is
mentioned in the commit message.

At baseline nothing freed ndp->vlan_vids, and ftgmac100_remove() calls
ncsi_unregister_dev() before unregister_netdev(), so those entries were
leaked; the requests[] walk additionally releases still-held cmd/rsp SKBs
through ncsi_free_request() -> consume_skb(cmd)/consume_skb(rsp).  Only the
nr->timer cancellation seems required by the new ordering, since nr->timer
is embedded in ndp.

Would it make sense to split these into separate patches, given they all
land under the single "Fixes: 2d283bdd079c" tag?

> +
> +	list_for_each_entry_safe(np, tmp, &ndp->packages, node)
> +		ncsi_remove_package(np);
> +
>  	kfree(ndp);
>  }
>  EXPORT_SYMBOL_GPL(ncsi_unregister_dev);

> diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c
> index 8cc538358f6a3..f8ea2ea73fb0d 100644
> --- a/net/ncsi/ncsi-netlink.c
> +++ b/net/ncsi/ncsi-netlink.c

[ ... ]

> @@ -228,11 +236,14 @@ static int ncsi_pkg_info_all_nl(struct sk_buff *skb,
>  	if (!attrs[NCSI_ATTR_IFINDEX])
>  		return -EINVAL;
>  
> +	rcu_read_lock();
>  	ndp = ndp_from_ifindex(get_net(sock_net(skb->sk)),
>  			       nla_get_u32(attrs[NCSI_ATTR_IFINDEX]));
>  
> -	if (!ndp)
> +	if (!ndp) {
> +		rcu_read_unlock();
>  		return -ENODEV;
> +	}

[Severity: High]
This is a pre-existing issue and not introduced by this patch, but since
new return paths are being added right next to it: does this get_net() leak
a struct net reference?

ndp_from_ifindex() treats its struct net * as borrowed and never drops it:

net/ncsi/ncsi-netlink.c:ndp_from_ifindex() {
	dev = dev_get_by_index(net, ifindex);
	...
	dev_put(dev);
	return ndp;
}

git grep put_net over net/ncsi/ finds nothing.  ncsi_pkg_info_nl() passes
genl_info_net(info) with no reference taken, while ncsi_pkg_info_all_nl(),
ncsi_set_interface_nl(), ncsi_clear_interface_nl(), ncsi_send_cmd_nl(),
ncsi_set_package_mask_nl() and ncsi_set_channel_mask_nl() all pass
get_net(sock_net(...)).  Since get_net() runs before the !ndp test, the
reference is pinned even on the new early -ENODEV return above, and
NCSI_CMD_PKG_INFO carries no GENL_ADMIN_PERM so the dump path is reachable
unprivileged.

Would dropping the get_net() at these sites (matching ncsi_pkg_info_nl) or
adding put_net() in the helper be the right fix, so the peer call sites
agree on who owns the reference?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921153633.565779-1-wongboonjhee52%40gmail.com