diff mbox series

[net] devlink: fix netns refcount leak in devlink_nl_cmd_reload()

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

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 2 this patch: 2
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 20 this patch: 20
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 4 this patch: 4
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 28 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Eric Dumazet Dec. 5, 2021, 7:28 p.m. UTC 
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(-)

Comments

Leon Romanovsky Dec. 6, 2021, 7:16 a.m. UTC | #1 
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>
patchwork-bot+netdevbpf@kernel.org Dec. 7, 2021, 1:10 a.m. UTC | #2 
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 mbox series

Patch

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)