Message ID | 20211205192822.1741045-1-eric.dumazet@gmail.com (mailing list archive) |
---|---|
State | Accepted |
Commit | 4dbb0dad8e63fcd0b5a117c2861d2abe7ff5f186 |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | [net] devlink: fix netns refcount leak in devlink_nl_cmd_reload() | expand |
On Sun, Dec 05, 2021 at 11:28:22AM -0800, Eric Dumazet wrote: > From: Eric Dumazet <edumazet@google.com> > > While preparing my patch series adding netns refcount tracking, > I spotted bugs in devlink_nl_cmd_reload() > > Some error paths forgot to release a refcount on a netns. > > To fix this, we can reduce the scope of get_net()/put_net() > section around the call to devlink_reload(). > > Fixes: ccdf07219da6 ("devlink: Add reload action option to devlink reload command") > Fixes: dc64cc7c6310 ("devlink: Add devlink reload limit option") > Signed-off-by: Eric Dumazet <edumazet@google.com> > Cc: Moshe Shemesh <moshe@mellanox.com> > Cc: Jakub Kicinski <kuba@kernel.org> > Cc: Jacob Keller <jacob.e.keller@intel.com> > Cc: Jiri Pirko <jiri@nvidia.com> > --- > net/core/devlink.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > Thanks, Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Hello: This patch was applied to netdev/net.git (master) by Jakub Kicinski <kuba@kernel.org>: On Sun, 5 Dec 2021 11:28:22 -0800 you wrote: > From: Eric Dumazet <edumazet@google.com> > > While preparing my patch series adding netns refcount tracking, > I spotted bugs in devlink_nl_cmd_reload() > > Some error paths forgot to release a refcount on a netns. > > [...] Here is the summary with links: - [net] devlink: fix netns refcount leak in devlink_nl_cmd_reload() https://git.kernel.org/netdev/net/c/4dbb0dad8e63 You are awesome, thank you!
diff --git a/net/core/devlink.c b/net/core/devlink.c index 5ad72dbfcd0797ae045734b83fbbdc090ae3ff53..c06c9ba6e8c5ea00a3999700a6724a404c1f05f9 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -4110,14 +4110,6 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info) return err; } - if (info->attrs[DEVLINK_ATTR_NETNS_PID] || - info->attrs[DEVLINK_ATTR_NETNS_FD] || - info->attrs[DEVLINK_ATTR_NETNS_ID]) { - dest_net = devlink_netns_get(skb, info); - if (IS_ERR(dest_net)) - return PTR_ERR(dest_net); - } - if (info->attrs[DEVLINK_ATTR_RELOAD_ACTION]) action = nla_get_u8(info->attrs[DEVLINK_ATTR_RELOAD_ACTION]); else @@ -4160,6 +4152,14 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info) return -EINVAL; } } + if (info->attrs[DEVLINK_ATTR_NETNS_PID] || + info->attrs[DEVLINK_ATTR_NETNS_FD] || + info->attrs[DEVLINK_ATTR_NETNS_ID]) { + dest_net = devlink_netns_get(skb, info); + if (IS_ERR(dest_net)) + return PTR_ERR(dest_net); + } + err = devlink_reload(devlink, dest_net, action, limit, &actions_performed, info->extack); if (dest_net)