From ba45106342bbdd905651cb9fcefb8c11871d4c25 Mon Sep 17 00:00:00 2001 From: Cosmin Ratiu Date: Tue, 16 Jun 2026 14:06:32 +0300 Subject: [PATCH 1/2] devlink: Fix parent ref leak in devl_rate_node_create() In the original commit the function bails out on kstrdup failure, forgetting to decrement the refcnt of the parent. Fix that by moving the parent refcnt setting after kstrdup. Fixes: caba177d7f4d ("devlink: Enable creation of the devlink-rate nodes from the driver") Signed-off-by: Cosmin Ratiu Reviewed-by: Carolina Jubran Reviewed-by: Simon Horman Link: https://patch.msgid.link/20260616110633.1449432-2-cratiu@nvidia.com Signed-off-by: Jakub Kicinski --- net/devlink/rate.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/net/devlink/rate.c b/net/devlink/rate.c index 41be2d6c2954..210e26c6cfa0 100644 --- a/net/devlink/rate.c +++ b/net/devlink/rate.c @@ -725,11 +725,6 @@ devl_rate_node_create(struct devlink *devlink, void *priv, char *node_name, if (!rate_node) return ERR_PTR(-ENOMEM); - if (parent) { - rate_node->parent = parent; - refcount_inc(&rate_node->parent->refcnt); - } - rate_node->type = DEVLINK_RATE_TYPE_NODE; rate_node->devlink = devlink; rate_node->priv = priv; @@ -740,6 +735,11 @@ devl_rate_node_create(struct devlink *devlink, void *priv, char *node_name, return ERR_PTR(-ENOMEM); } + if (parent) { + rate_node->parent = parent; + refcount_inc(&rate_node->parent->refcnt); + } + refcount_set(&rate_node->refcnt, 1); list_add(&rate_node->list, &devlink->rate_list); devlink_rate_notify(rate_node, DEVLINK_CMD_RATE_NEW); From ba81a8b80f042038b9f73a4e5bb135de890b59bb Mon Sep 17 00:00:00 2001 From: Cosmin Ratiu Date: Tue, 16 Jun 2026 14:06:33 +0300 Subject: [PATCH 2/2] devlink: Fix parent ref leak on tc-bw failure When a node is created via rate-new with tc-bw and a parent node, devlink_nl_rate_set() executes the sequence of ops. It bails out on the first failure and doesn't rollback anything. For most things that is fine (setting some numbers), but the parent set can leak if there's another failure after that. That is precisely what happens when parent setting isn't the last block in the function. After the referenced "Fixes" commit, when tc-bw fails to be set the function bails out after having set the parent and incremented its refcount. There are two callers: - devlink_nl_rate_set_doit() is fine, it just reports the error. - but devlink_nl_rate_new_doit() frees the newly created node and leaks the parent refcnt. Fix that by reordering the blocks so parent setting is last and adding a comment explaining this so future modification preserve the ordering (hopefully). Fixes: 566e8f108fc7 ("devlink: Extend devlink rate API with traffic classes bandwidth management") Signed-off-by: Cosmin Ratiu Reviewed-by: Carolina Jubran Reviewed-by: Simon Horman Link: https://patch.msgid.link/20260616110633.1449432-3-cratiu@nvidia.com Signed-off-by: Jakub Kicinski --- net/devlink/rate.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/net/devlink/rate.c b/net/devlink/rate.c index 210e26c6cfa0..533d21b028a7 100644 --- a/net/devlink/rate.c +++ b/net/devlink/rate.c @@ -486,16 +486,19 @@ static int devlink_nl_rate_set(struct devlink_rate *devlink_rate, devlink_rate->tx_weight = weight; } - nla_parent = attrs[DEVLINK_ATTR_RATE_PARENT_NODE_NAME]; - if (nla_parent) { - err = devlink_nl_rate_parent_node_set(devlink_rate, info, - nla_parent); + if (attrs[DEVLINK_ATTR_RATE_TC_BWS]) { + err = devlink_nl_rate_tc_bw_set(devlink_rate, info); if (err) return err; } - if (attrs[DEVLINK_ATTR_RATE_TC_BWS]) { - err = devlink_nl_rate_tc_bw_set(devlink_rate, info); + /* Keep parent setting last because it takes a reference. This function + * has no rollback, so failing after taking the ref would leak it. + */ + nla_parent = attrs[DEVLINK_ATTR_RATE_PARENT_NODE_NAME]; + if (nla_parent) { + err = devlink_nl_rate_parent_node_set(devlink_rate, info, + nla_parent); if (err) return err; }