From c75b6f6eaacd0b74b832414cc3b9289c3686e941 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Fri, 22 May 2026 16:06:42 -0700 Subject: [PATCH 1/6] ethtool: rss: avoid modifying the RSS context response Gemini says that we're modifying the RSS_CREATE response skb. I think it's right, the comment says that unicast() should unshare the skb but I'm not entirely sure what I meant there. netlink_trim() does a copy but only if skb is not well sized (it's at least 2x larger than necessary for the payload). Fixes: a166ab7816c5 ("ethtool: rss: support creating contexts via Netlink") Link: https://patch.msgid.link/20260522230647.1705600-2-kuba@kernel.org Signed-off-by: Jakub Kicinski --- net/ethtool/rss.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/net/ethtool/rss.c b/net/ethtool/rss.c index 353110b862ab..8ffec9785efa 100644 --- a/net/ethtool/rss.c +++ b/net/ethtool/rss.c @@ -981,11 +981,17 @@ ethnl_rss_create_validate(struct net_device *dev, struct genl_info *info) } static void -ethnl_rss_create_send_ntf(struct sk_buff *rsp, struct net_device *dev) +ethnl_rss_create_send_ntf(const struct sk_buff *rsp, struct net_device *dev) { - struct nlmsghdr *nlh = (void *)rsp->data; struct genlmsghdr *genl_hdr; + struct nlmsghdr *nlh; + struct sk_buff *ntf; + ntf = skb_copy_expand(rsp, 0, 0, GFP_KERNEL); + if (!ntf) + return; + + nlh = nlmsg_hdr(ntf); /* Convert the reply into a notification */ nlh->nlmsg_pid = 0; nlh->nlmsg_seq = ethnl_bcast_seq_next(); @@ -993,7 +999,7 @@ ethnl_rss_create_send_ntf(struct sk_buff *rsp, struct net_device *dev) genl_hdr = nlmsg_data(nlh); genl_hdr->cmd = ETHTOOL_MSG_RSS_CREATE_NTF; - ethnl_multicast(rsp, dev); + ethnl_multicast(ntf, dev); } int ethnl_rss_create_doit(struct sk_buff *skb, struct genl_info *info) @@ -1104,12 +1110,8 @@ int ethnl_rss_create_doit(struct sk_buff *skb, struct genl_info *info) genlmsg_end(rsp, hdr); - /* Use the same skb for the response and the notification, - * genlmsg_reply() will copy the skb if it has elevated user count. - */ - skb_get(rsp); - ret = genlmsg_reply(rsp, info); ethnl_rss_create_send_ntf(rsp, dev); + ret = genlmsg_reply(rsp, info); rsp = NULL; exit_unlock: From 3e6c6e9782ff8a8d8ded774b07ad4590cd61d04c Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Fri, 22 May 2026 16:06:43 -0700 Subject: [PATCH 2/6] ethtool: rss: add missing errno on RSS context delete Remember to set ret before jumping out if someone tries to delete a context on a device which doesn't support contexts. Fixes: fbe09277fa63 ("ethtool: rss: support removing contexts via Netlink") Link: https://patch.msgid.link/20260522230647.1705600-3-kuba@kernel.org Signed-off-by: Jakub Kicinski --- net/ethtool/rss.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/net/ethtool/rss.c b/net/ethtool/rss.c index 8ffec9785efa..a16ee1e8e640 100644 --- a/net/ethtool/rss.c +++ b/net/ethtool/rss.c @@ -1170,8 +1170,10 @@ int ethnl_rss_delete_doit(struct sk_buff *skb, struct genl_info *info) dev = req.dev; ops = dev->ethtool_ops; - if (!ops->create_rxfh_context) + if (!ops->create_rxfh_context) { + ret = -EOPNOTSUPP; goto exit_free_dev; + } rtnl_lock(); netdev_lock_ops(dev); From 8d60141a32875248ef71d49c9920fa5e2aa40b29 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Fri, 22 May 2026 16:06:44 -0700 Subject: [PATCH 3/6] ethtool: rss: fix falsely ignoring indir table updates rss_set_prep_indir() compares the new indirection table against the current one to determine whether any update is needed. The memcmp call passes data->indir_size as the length argument, but indir_size is the number of u32 entries, not the byte count. Fixes: c0ae03588bbb ("ethtool: rss: initial RSS_SET (indirection table handling)") Link: https://patch.msgid.link/20260522230647.1705600-4-kuba@kernel.org Signed-off-by: Jakub Kicinski --- net/ethtool/rss.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/ethtool/rss.c b/net/ethtool/rss.c index a16ee1e8e640..458a4a7907e4 100644 --- a/net/ethtool/rss.c +++ b/net/ethtool/rss.c @@ -686,7 +686,7 @@ rss_set_prep_indir(struct net_device *dev, struct genl_info *info, ethtool_rxfh_indir_default(i, num_rx_rings); } - *mod |= memcmp(rxfh->indir, data->indir_table, data->indir_size); + *mod |= memcmp(rxfh->indir, data->indir_table, alloc_size); return user_size; From 266297692f97008ca48bc311775c087c59bd7fe3 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Fri, 22 May 2026 16:06:45 -0700 Subject: [PATCH 4/6] ethtool: rss: fix indir_table and hkey leak on get_rxfh failure rss_prepare_get() allocates the indirection table and hash key buffer via rss_get_data_alloc(), then calls ops->get_rxfh() to populate them. If get_rxfh() fails, the function returns an error without freeing the allocation. Fixes: 4f038a6a02d2 ("net: ethtool: Don't call .cleanup_data when prepare_data fails") Link: https://patch.msgid.link/20260522230647.1705600-5-kuba@kernel.org Signed-off-by: Jakub Kicinski --- net/ethtool/rss.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/net/ethtool/rss.c b/net/ethtool/rss.c index 458a4a7907e4..9fb675d29232 100644 --- a/net/ethtool/rss.c +++ b/net/ethtool/rss.c @@ -170,8 +170,10 @@ rss_prepare_get(const struct rss_req_info *request, struct net_device *dev, rxfh.key = data->hkey; ret = ops->get_rxfh(dev, &rxfh); - if (ret) + if (ret) { + rss_get_data_free(data); goto out_unlock; + } data->hfunc = rxfh.hfunc; data->input_xfrm = rxfh.input_xfrm; From 78ccf1a70c6378e1f5073a8c2209b5129067b925 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Fri, 22 May 2026 16:06:46 -0700 Subject: [PATCH 5/6] ethtool: rss: fix hkey leak when indir_size is 0 rss_get_data_alloc() allocates a single buffer that backs both the indirection table and the hash key, but only assigned data->indir_table when indir_size was nonzero. The expectation was that no driver implements RSS without supporting indirection table but apparently enic does just that (it's the only such in-tree driver). enic has get_rxfh_key_size but no get_rxfh_indir_size. data->indir_table stays as NULL, hkey gets set but rss_get_data_free() kfree(data->indir_table) is a nop and the allocation leaks. Always store the allocation base in data->indir_table so the free path is unambiguous. No caller treats indir_table as a sentinel; everything keys off indir_size. Fixes: 7112a04664bf ("ethtool: add netlink based get rss support") Link: https://patch.msgid.link/20260522230647.1705600-6-kuba@kernel.org Signed-off-by: Jakub Kicinski --- net/ethtool/rss.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/net/ethtool/rss.c b/net/ethtool/rss.c index 9fb675d29232..f5cf214f8f85 100644 --- a/net/ethtool/rss.c +++ b/net/ethtool/rss.c @@ -134,8 +134,7 @@ rss_get_data_alloc(struct net_device *dev, struct rss_reply_data *data) if (!rss_config) return -ENOMEM; - if (data->indir_size) - data->indir_table = (u32 *)rss_config; + data->indir_table = (u32 *)rss_config; if (data->hkey_size) data->hkey = rss_config + indir_bytes; From 32a9ecde62731c9f7412507709192c84dafc38d1 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Fri, 22 May 2026 16:06:47 -0700 Subject: [PATCH 6/6] ethtool: rss: avoid device context leak on reply-build failure We wait with filling the reply for new RSS context creation until after the driver ->create_rxfh_context call. The driver needs to fill some of the defaults in the context. The failure of rss_fill_reply() is somewhat theoretical, but doesn't take much effort to handle it properly. Call ->remove_rxfh_context(). If the driver's remove callback fails (some implementations like sfc can return real command errors from firmware RPCs) - skip the xa_erase and kfree, leaving the context in the xarray. This matches how ethnl_rss_delete_doit() behaves. Fixes: a166ab7816c5 ("ethtool: rss: support creating contexts via Netlink") Link: https://patch.msgid.link/20260522230647.1705600-7-kuba@kernel.org Signed-off-by: Jakub Kicinski --- net/ethtool/rss.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/net/ethtool/rss.c b/net/ethtool/rss.c index f5cf214f8f85..53792f53f922 100644 --- a/net/ethtool/rss.c +++ b/net/ethtool/rss.c @@ -1106,7 +1106,7 @@ int ethnl_rss_create_doit(struct sk_buff *skb, struct genl_info *info) ntf_fail |= rss_fill_reply(rsp, &req.base, &data.base); if (WARN_ON(!hdr || ntf_fail)) { ret = -EMSGSIZE; - goto exit_unlock; + goto err_remove_ctx; } genlmsg_end(rsp, hdr); @@ -1134,6 +1134,10 @@ int ethnl_rss_create_doit(struct sk_buff *skb, struct genl_info *info) nlmsg_free(rsp); return ret; +err_remove_ctx: + if (ops->remove_rxfh_context(dev, ctx, req.rss_context, NULL)) + /* leave the context on failure, like ethnl_rss_delete_doit() */ + goto exit_unlock; err_ctx_id_free: xa_erase(&dev->ethtool->rss_ctx, req.rss_context); err_unlock_free_ctx: