diff mbox series

[v3,net] af_unix: Do not call kmemdup() for init_net's sysctl table.

Message ID 20220627233627.51646-1-kuniyu@amazon.com (mailing list archive)
State Accepted
Commit 849d5aa3a1d833d0b3a18c2d5b816e23003cfe55
Delegated to: Netdev Maintainers
Headers show
Series [v3,net] af_unix: Do not call kmemdup() for init_net's sysctl table. | 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: 0 this patch: 0
netdev/cc_maintainers warning 1 maintainers not CCed: joannekoong@fb.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 36 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Kuniyuki Iwashima June 27, 2022, 11:36 p.m. UTC
While setting up init_net's sysctl table, we need not duplicate the
global table and can use it directly as ipv4_sysctl_init_net() does.

Unlike IPv4, AF_UNIX does not have a huge sysctl table for now, so it
cannot be a problem, but this patch makes code consistent.

Fixes: 1597fbc0faf8 ("[UNIX]: Make the unix sysctl tables per-namespace")
Acked-by: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
v3:
  * Update changelog
  * Fix a bug when we unload unix.ko

v2: https://lore.kernel.org/all/20220626082331.36119-1-kuniyu@amazon.com/
  * Fix NULL comparison style by checkpatch.pl

v1: https://lore.kernel.org/all/20220626074454.28944-1-kuniyu@amazon.com/
---
 net/unix/sysctl_net_unix.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

Comments

Jakub Kicinski June 29, 2022, 3:51 a.m. UTC | #1
On Mon, 27 Jun 2022 16:36:27 -0700 Kuniyuki Iwashima wrote:
> While setting up init_net's sysctl table, we need not duplicate the
> global table and can use it directly as ipv4_sysctl_init_net() does.
> 
> Unlike IPv4, AF_UNIX does not have a huge sysctl table for now, so it
> cannot be a problem, but this patch makes code consistent.

Thanks for the extra info. It sounds like an optimization, tho.
We save one table's worth of memory. Any objections to applying
this to net-next?

> Fixes: 1597fbc0faf8 ("[UNIX]: Make the unix sysctl tables per-namespace")
> Acked-by: Eric W. Biederman <ebiederm@xmission.com>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Kuniyuki Iwashima June 29, 2022, 3:55 a.m. UTC | #2
From:   Jakub Kicinski <kuba@kernel.org>
Date:   Tue, 28 Jun 2022 20:51:25 -0700
> On Mon, 27 Jun 2022 16:36:27 -0700 Kuniyuki Iwashima wrote:
> > While setting up init_net's sysctl table, we need not duplicate the
> > global table and can use it directly as ipv4_sysctl_init_net() does.
> > 
> > Unlike IPv4, AF_UNIX does not have a huge sysctl table for now, so it
> > cannot be a problem, but this patch makes code consistent.
> 
> Thanks for the extra info. It sounds like an optimization, tho.
> We save one table's worth of memory. Any objections to applying
> this to net-next?

I'm fine with net-next.

Thank you,
Kuniyuki


> > Fixes: 1597fbc0faf8 ("[UNIX]: Make the unix sysctl tables per-namespace")
> > Acked-by: Eric W. Biederman <ebiederm@xmission.com>
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
patchwork-bot+netdevbpf@kernel.org June 29, 2022, 4:10 a.m. UTC | #3
Hello:

This patch was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 27 Jun 2022 16:36:27 -0700 you wrote:
> While setting up init_net's sysctl table, we need not duplicate the
> global table and can use it directly as ipv4_sysctl_init_net() does.
> 
> Unlike IPv4, AF_UNIX does not have a huge sysctl table for now, so it
> cannot be a problem, but this patch makes code consistent.
> 
> Fixes: 1597fbc0faf8 ("[UNIX]: Make the unix sysctl tables per-namespace")
> Acked-by: Eric W. Biederman <ebiederm@xmission.com>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> 
> [...]

Here is the summary with links:
  - [v3,net] af_unix: Do not call kmemdup() for init_net's sysctl table.
    https://git.kernel.org/netdev/net-next/c/849d5aa3a1d8

You are awesome, thank you!
Eric Dumazet July 8, 2022, 4:10 p.m. UTC | #4
On 6/28/22 01:36, Kuniyuki Iwashima wrote:
> While setting up init_net's sysctl table, we need not duplicate the
> global table and can use it directly as ipv4_sysctl_init_net() does.
>
> Unlike IPv4, AF_UNIX does not have a huge sysctl table for now, so it
> cannot be a problem, but this patch makes code consistent.
>
> Fixes: 1597fbc0faf8 ("[UNIX]: Make the unix sysctl tables per-namespace")
> Acked-by: Eric W. Biederman <ebiederm@xmission.com>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> ---
> v3:
>    * Update changelog
>    * Fix a bug when we unload unix.ko
>
> v2: https://lore.kernel.org/all/20220626082331.36119-1-kuniyu@amazon.com/
>    * Fix NULL comparison style by checkpatch.pl
>
> v1: https://lore.kernel.org/all/20220626074454.28944-1-kuniyu@amazon.com/
> ---
>   net/unix/sysctl_net_unix.c | 19 +++++++++++++------
>   1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/net/unix/sysctl_net_unix.c b/net/unix/sysctl_net_unix.c
> index 01d44e2598e2..3f1fdffd6092 100644
> --- a/net/unix/sysctl_net_unix.c
> +++ b/net/unix/sysctl_net_unix.c
> @@ -26,11 +26,16 @@ int __net_init unix_sysctl_register(struct net *net)
>   {
>   	struct ctl_table *table;
>   
> -	table = kmemdup(unix_table, sizeof(unix_table), GFP_KERNEL);
> -	if (table == NULL)
> -		goto err_alloc;
> +	if (net_eq(net, &init_net)) {
> +		table = unix_table;
> +	} else {
> +		table = kmemdup(unix_table, sizeof(unix_table), GFP_KERNEL);
> +		if (!table)
> +			goto err_alloc;
> +
> +		table[0].data = &net->unx.sysctl_max_dgram_qlen;
> +	}
>   
> -	table[0].data = &net->unx.sysctl_max_dgram_qlen;
>   	net->unx.ctl = register_net_sysctl(net, "net/unix", table);
>   	if (net->unx.ctl == NULL)
>   		goto err_reg;
> @@ -38,7 +43,8 @@ int __net_init unix_sysctl_register(struct net *net)
>   	return 0;
>   
>   err_reg:
> -	kfree(table);
> +	if (net_eq(net, &init_net))
> +		kfree(table);

Inverted test, I guess :/

I will send a fix.


>   err_alloc:
>   	return -ENOMEM;
>   }
> @@ -49,5 +55,6 @@ void unix_sysctl_unregister(struct net *net)
>   
>   	table = net->unx.ctl->ctl_table_arg;
>   	unregister_net_sysctl_table(net->unx.ctl);
> -	kfree(table);
> +	if (!net_eq(net, &init_net))
> +		kfree(table);
>   }
diff mbox series

Patch

diff --git a/net/unix/sysctl_net_unix.c b/net/unix/sysctl_net_unix.c
index 01d44e2598e2..3f1fdffd6092 100644
--- a/net/unix/sysctl_net_unix.c
+++ b/net/unix/sysctl_net_unix.c
@@ -26,11 +26,16 @@  int __net_init unix_sysctl_register(struct net *net)
 {
 	struct ctl_table *table;
 
-	table = kmemdup(unix_table, sizeof(unix_table), GFP_KERNEL);
-	if (table == NULL)
-		goto err_alloc;
+	if (net_eq(net, &init_net)) {
+		table = unix_table;
+	} else {
+		table = kmemdup(unix_table, sizeof(unix_table), GFP_KERNEL);
+		if (!table)
+			goto err_alloc;
+
+		table[0].data = &net->unx.sysctl_max_dgram_qlen;
+	}
 
-	table[0].data = &net->unx.sysctl_max_dgram_qlen;
 	net->unx.ctl = register_net_sysctl(net, "net/unix", table);
 	if (net->unx.ctl == NULL)
 		goto err_reg;
@@ -38,7 +43,8 @@  int __net_init unix_sysctl_register(struct net *net)
 	return 0;
 
 err_reg:
-	kfree(table);
+	if (net_eq(net, &init_net))
+		kfree(table);
 err_alloc:
 	return -ENOMEM;
 }
@@ -49,5 +55,6 @@  void unix_sysctl_unregister(struct net *net)
 
 	table = net->unx.ctl->ctl_table_arg;
 	unregister_net_sysctl_table(net->unx.ctl);
-	kfree(table);
+	if (!net_eq(net, &init_net))
+		kfree(table);
 }