net: shaper: reject duplicate leaves in GROUP request

net_shaper_nl_group_doit() does not deduplicate NET_SHAPER_A_LEAVES
entries. When userspace supplies the same leaf handle twice, the same
old-parent pointer lands twice in old_nodes[]. The cleanup loop double
frees the parent. Of course the same parent may still be in old_nodes[]
twice if we are moving multiple of its leaves.

Note that this patch also implicitly fixes the fact that the
i >= leaves_count path forgets to set ret.

Fixes: 5d5d4700e7 ("net-shapers: implement NL group operation")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Link: https://patch.msgid.link/20260510192904.3987113-4-kuba@kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
This commit is contained in:
Jakub Kicinski 2026-05-10 12:28:57 -07:00 committed by Paolo Abeni
parent 235fb53761
commit a9a2fa1da6

View File

@ -941,6 +941,46 @@ static int net_shaper_handle_cmp(const struct net_shaper_handle *a,
return memcmp(a, b, sizeof(*a));
}
static int net_shaper_parse_leaves(struct net_shaper_binding *binding,
struct genl_info *info,
const struct net_shaper *node,
struct net_shaper *leaves,
int leaves_count)
{
struct nlattr *attr;
int i, j, ret, rem;
i = 0;
nla_for_each_attr_type(attr, NET_SHAPER_A_LEAVES,
genlmsg_data(info->genlhdr),
genlmsg_len(info->genlhdr), rem) {
if (WARN_ON_ONCE(i >= leaves_count))
return -EINVAL;
ret = net_shaper_parse_leaf(binding, attr, info,
node, &leaves[i]);
if (ret)
return ret;
/* Reject duplicates */
for (j = 0; j < i; j++) {
if (net_shaper_handle_cmp(&leaves[i].handle,
&leaves[j].handle))
continue;
NL_SET_ERR_MSG_ATTR_FMT(info->extack, attr,
"Duplicate leaf shaper %d:%d",
leaves[i].handle.scope,
leaves[i].handle.id);
return -EINVAL;
}
i++;
}
return 0;
}
static int net_shaper_parent_from_leaves(int leaves_count,
const struct net_shaper *leaves,
struct net_shaper *node,
@ -1197,10 +1237,9 @@ int net_shaper_nl_group_doit(struct sk_buff *skb, struct genl_info *info)
struct net_shaper **old_nodes, *leaves, node = {};
struct net_shaper_hierarchy *hierarchy;
struct net_shaper_binding *binding;
int i, ret, rem, leaves_count;
int i, ret, leaves_count;
int old_nodes_count = 0;
struct sk_buff *msg;
struct nlattr *attr;
if (GENL_REQ_ATTR_CHECK(info, NET_SHAPER_A_LEAVES))
return -EINVAL;
@ -1228,19 +1267,10 @@ int net_shaper_nl_group_doit(struct sk_buff *skb, struct genl_info *info)
if (ret)
goto free_leaves;
i = 0;
nla_for_each_attr_type(attr, NET_SHAPER_A_LEAVES,
genlmsg_data(info->genlhdr),
genlmsg_len(info->genlhdr), rem) {
if (WARN_ON_ONCE(i >= leaves_count))
goto free_leaves;
ret = net_shaper_parse_leaf(binding, attr, info,
&node, &leaves[i]);
if (ret)
goto free_leaves;
i++;
}
ret = net_shaper_parse_leaves(binding, info, &node,
leaves, leaves_count);
if (ret)
goto free_leaves;
/* Prepare the msg reply in advance, to avoid device operation
* rollback on allocation failure.