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 --- 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; }