All of lore.kernel.org
 help / color / mirror / Atom feed
* [REGRESSION] netfilter: conntrack: Unable to change conntrack accounting of a net namespace via 'nf_conntrack_acct' sysfs
@ 2019-08-27 10:57 Shmulik Ladkani
  2019-08-27 11:18 ` Florian Westphal
  2019-08-27 11:24 ` [PATCH nf] netfilter: conntrack: make sysctls per-namespace again Florian Westphal
  0 siblings, 2 replies; 6+ messages in thread
From: Shmulik Ladkani @ 2019-08-27 10:57 UTC (permalink / raw)
  To: Florian Westphal, Pablo Neira Ayuso; +Cc: netdev, shmulik

Hi,

Prior d912dec12428 ("netfilter: conntrack: merge acct and helper sysctl table with main one")
one was able to enable extended accounting within a (non-init)
net-namespace by setting: 'net.netfilter.nf_conntrack_acct=1'

However since d912dec12428, doing so results in changing init_net's
sysctl_acct field, instead of the relevant net's sysctl_acct.

Seen in original code, PRE d912dec12428, which creates a reference to
each net's _OWN_ ct.sysctl_acct within a separate acct_sysctl_table,
snip:

-static int nf_conntrack_acct_init_sysctl(struct net *net)
-{
-	struct ctl_table *table;
-
-	table = kmemdup(acct_sysctl_table, sizeof(acct_sysctl_table),
-			GFP_KERNEL);
-	if (!table)
-		goto out;
-
-	table[0].data = &net->ct.sysctl_acct;
-

(where 'nf_conntrack_acct_init_sysctl()' was originally called by
'nf_conntrack_acct_pernet_init()').

However POST d912dec12428, the per-net netfilter sysctl table simply
inherits from global 'nf_ct_sysctl_table[]', which has 

+		.data		= &init_net.ct.sysctl_acct,

effectivly making any 'net.netfilter.nf_conntrack_acct' sysctl change
affect the 'init_net' and not relevant net namespace.

Also, looks like "nf_conntrack_helper", "nf_conntrack_events",
"nf_conntrack_timestamp" where also harmed in a similar way, see:

  d912dec12428 ("netfilter: conntrack: merge acct and helper sysctl table with main one")
  cb2833ed0044 ("netfilter: conntrack: merge ecache and timestamp sysctl tables with main one")

Florian, would it be possible for you to revert these on -net ?

Many thanks,
Shmulik

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [REGRESSION] netfilter: conntrack: Unable to change conntrack accounting of a net namespace via 'nf_conntrack_acct' sysfs
  2019-08-27 10:57 [REGRESSION] netfilter: conntrack: Unable to change conntrack accounting of a net namespace via 'nf_conntrack_acct' sysfs Shmulik Ladkani
@ 2019-08-27 11:18 ` Florian Westphal
  2019-08-27 11:24 ` [PATCH nf] netfilter: conntrack: make sysctls per-namespace again Florian Westphal
  1 sibling, 0 replies; 6+ messages in thread
From: Florian Westphal @ 2019-08-27 11:18 UTC (permalink / raw)
  To: Shmulik Ladkani; +Cc: Florian Westphal, Pablo Neira Ayuso, netdev, shmulik

Shmulik Ladkani <shmulik.ladkani@gmail.com> wrote:
> -static int nf_conntrack_acct_init_sysctl(struct net *net)
> -{
> -	struct ctl_table *table;
> -
> -	table = kmemdup(acct_sysctl_table, sizeof(acct_sysctl_table),
> -			GFP_KERNEL);
> -	if (!table)
> -		goto out;
> -
> -	table[0].data = &net->ct.sysctl_acct;
> -
> 
> (where 'nf_conntrack_acct_init_sysctl()' was originally called by
> 'nf_conntrack_acct_pernet_init()').
> 
> However POST d912dec12428, the per-net netfilter sysctl table simply
> inherits from global 'nf_ct_sysctl_table[]', which has 
> 
> +		.data		= &init_net.ct.sysctl_acct,
> 
> effectivly making any 'net.netfilter.nf_conntrack_acct' sysctl change
> affect the 'init_net' and not relevant net namespace.
>
> Also, looks like "nf_conntrack_helper", "nf_conntrack_events",
> "nf_conntrack_timestamp" where also harmed in a similar way, see:
> 
>   d912dec12428 ("netfilter: conntrack: merge acct and helper sysctl table with main one")
>   cb2833ed0044 ("netfilter: conntrack: merge ecache and timestamp sysctl tables with main one")

Thanks for reporting this bug, I will submit a patch soon.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH nf] netfilter: conntrack: make sysctls per-namespace again
  2019-08-27 10:57 [REGRESSION] netfilter: conntrack: Unable to change conntrack accounting of a net namespace via 'nf_conntrack_acct' sysfs Shmulik Ladkani
  2019-08-27 11:18 ` Florian Westphal
@ 2019-08-27 11:24 ` Florian Westphal
  2019-08-27 11:49   ` Shmulik Ladkani
                     ` (2 more replies)
  1 sibling, 3 replies; 6+ messages in thread
From: Florian Westphal @ 2019-08-27 11:24 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal, Shmulik Ladkani

When I merged the extension sysctl tables with the main one I forgot to
reset them on netns creation.  They currently read/write init_net settings.

Fixes: d912dec12428 ("netfilter: conntrack: merge acct and helper sysctl table with main one")
Fixes: cb2833ed0044 ("netfilter: conntrack: merge ecache and timestamp sysctl tables with main one")
Reported-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 Shmulik, could you please check if this fixes the bug for you?
 Thanks!

 net/netfilter/nf_conntrack_standalone.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index e0d392cb3075..0006503d2da9 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -1037,8 +1037,13 @@ static int nf_conntrack_standalone_init_sysctl(struct net *net)
 	table[NF_SYSCTL_CT_COUNT].data = &net->ct.count;
 	table[NF_SYSCTL_CT_CHECKSUM].data = &net->ct.sysctl_checksum;
 	table[NF_SYSCTL_CT_LOG_INVALID].data = &net->ct.sysctl_log_invalid;
+	table[NF_SYSCTL_CT_ACCT].data = &net->ct.sysctl_acct;
+	table[NF_SYSCTL_CT_HELPER].data = &net->ct.sysctl_auto_assign_helper;
 #ifdef CONFIG_NF_CONNTRACK_EVENTS
 	table[NF_SYSCTL_CT_EVENTS].data = &net->ct.sysctl_events;
+#endif
+#ifdef CONFIG_NF_CONNTRACK_TIMESTAMP
+	table[NF_SYSCTL_CT_TIMESTAMP].data = &net->ct.sysctl_tstamp;
 #endif
 	table[NF_SYSCTL_CT_PROTO_TIMEOUT_GENERIC].data = &nf_generic_pernet(net)->timeout;
 	table[NF_SYSCTL_CT_PROTO_TIMEOUT_ICMP].data = &nf_icmp_pernet(net)->timeout;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH nf] netfilter: conntrack: make sysctls per-namespace again
  2019-08-27 11:24 ` [PATCH nf] netfilter: conntrack: make sysctls per-namespace again Florian Westphal
@ 2019-08-27 11:49   ` Shmulik Ladkani
  2019-08-27 15:45   ` Pablo Neira Ayuso
  2019-08-28  7:40   ` Shmulik Ladkani
  2 siblings, 0 replies; 6+ messages in thread
From: Shmulik Ladkani @ 2019-08-27 11:49 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

On Tue, 27 Aug 2019 13:24:52 +0200
Florian Westphal <fw@strlen.de> wrote:

> ---
>  Shmulik, could you please check if this fixes the bug for you?
>  Thanks!

Thanks Florian for the quick fix, I'll check this by tomorrow mytime.

Shmulik

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH nf] netfilter: conntrack: make sysctls per-namespace again
  2019-08-27 11:24 ` [PATCH nf] netfilter: conntrack: make sysctls per-namespace again Florian Westphal
  2019-08-27 11:49   ` Shmulik Ladkani
@ 2019-08-27 15:45   ` Pablo Neira Ayuso
  2019-08-28  7:40   ` Shmulik Ladkani
  2 siblings, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2019-08-27 15:45 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel, Shmulik Ladkani

On Tue, Aug 27, 2019 at 01:24:52PM +0200, Florian Westphal wrote:
> When I merged the extension sysctl tables with the main one I forgot to
> reset them on netns creation.  They currently read/write init_net settings.

Applied, thanks Florian.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH nf] netfilter: conntrack: make sysctls per-namespace again
  2019-08-27 11:24 ` [PATCH nf] netfilter: conntrack: make sysctls per-namespace again Florian Westphal
  2019-08-27 11:49   ` Shmulik Ladkani
  2019-08-27 15:45   ` Pablo Neira Ayuso
@ 2019-08-28  7:40   ` Shmulik Ladkani
  2 siblings, 0 replies; 6+ messages in thread
From: Shmulik Ladkani @ 2019-08-28  7:40 UTC (permalink / raw)
  To: Florian Westphal, Pablo Neira Ayuso; +Cc: netfilter-devel

On Tue, 27 Aug 2019 13:24:52 +0200
Florian Westphal <fw@strlen.de> wrote:

> When I merged the extension sysctl tables with the main one I forgot to
> reset them on netns creation.  They currently read/write init_net settings.
> 
> Fixes: d912dec12428 ("netfilter: conntrack: merge acct and helper sysctl table with main one")
> Fixes: cb2833ed0044 ("netfilter: conntrack: merge ecache and timestamp sysctl tables with main one")
> Reported-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  Shmulik, could you please check if this fixes the bug for you?
>  Thanks!

Tested-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-08-28  7:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-27 10:57 [REGRESSION] netfilter: conntrack: Unable to change conntrack accounting of a net namespace via 'nf_conntrack_acct' sysfs Shmulik Ladkani
2019-08-27 11:18 ` Florian Westphal
2019-08-27 11:24 ` [PATCH nf] netfilter: conntrack: make sysctls per-namespace again Florian Westphal
2019-08-27 11:49   ` Shmulik Ladkani
2019-08-27 15:45   ` Pablo Neira Ayuso
2019-08-28  7:40   ` Shmulik Ladkani

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.