netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] Netfilter fixes for net
@ 2015-10-19 18:22 Pablo Neira Ayuso
  2015-10-19 18:22 ` [PATCH 1/4] netfilter: fix Kconfig dependencies for nf_dup_ipv{4,6} Pablo Neira Ayuso
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2015-10-19 18:22 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

Hi David,

The following patchset contains four Netfilter fixes for net, they are:

1) Fix Kconfig dependencies of new nf_dup_ipv4 and nf_dup_ipv6.

2) Remove bogus test nh_scope in IPv4 rpfilter match that is breaking
   --accept-local, from Xin Long.

3) Wait for RCU grace period after dropping the pending packets in the
   nfqueue, from Florian Westphal.

4) Fix sleeping allocation while holding spin_lock_bh, from Nikolay Borisov.

You can pull these changes from:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git

Thanks!

----------------------------------------------------------------

The following changes since commit b84f78782052ee4516903e5d0566a5eee365b771:

  net: Initialize flow flags in input path (2015-09-29 21:52:32 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git HEAD

for you to fetch changes up to 00db674bedd68ff8b5afae9030ff5e04d45d1b4a:

  netfilter: ipset: Fix sleeping memory allocation in atomic context (2015-10-17 13:01:24 +0200)

----------------------------------------------------------------
Florian Westphal (1):
      netfilter: sync with packet rx also after removing queue entries

Nikolay Borisov (1):
      netfilter: ipset: Fix sleeping memory allocation in atomic context

Pablo Neira Ayuso (1):
      netfilter: fix Kconfig dependencies for nf_dup_ipv{4,6}

lucien (1):
      netfilter: ipt_rpfilter: remove the nh_scope test in rpfilter_lookup_reverse

 net/ipv4/netfilter/Kconfig            | 1 +
 net/ipv4/netfilter/ipt_rpfilter.c     | 4 +---
 net/ipv6/netfilter/Kconfig            | 1 +
 net/netfilter/core.c                  | 2 ++
 net/netfilter/ipset/ip_set_list_set.c | 2 +-
 5 files changed, 6 insertions(+), 4 deletions(-)

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

* [PATCH 1/4] netfilter: fix Kconfig dependencies for nf_dup_ipv{4,6}
  2015-10-19 18:22 [PATCH 0/4] Netfilter fixes for net Pablo Neira Ayuso
@ 2015-10-19 18:22 ` Pablo Neira Ayuso
  2015-10-19 18:22 ` [PATCH 2/4] netfilter: ipt_rpfilter: remove the nh_scope test in rpfilter_lookup_reverse Pablo Neira Ayuso
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2015-10-19 18:22 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

net/built-in.o: In function `nf_dup_ipv4': (.text+0xed24d): undefined reference to `nf_conntrack_untracked'
net/built-in.o: In function `nf_dup_ipv4': (.text+0xed267): undefined reference to `nf_conntrack_untracked'
net/built-in.o: In function `nf_dup_ipv6': (.text+0x158aef): undefined reference to `nf_conntrack_untracked'
net/built-in.o: In function `nf_dup_ipv6': (.text+0x158b09): undefined reference to `nf_conntrack_untracked'

Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/ipv4/netfilter/Kconfig | 1 +
 net/ipv6/netfilter/Kconfig | 1 +
 2 files changed, 2 insertions(+)

diff --git a/net/ipv4/netfilter/Kconfig b/net/ipv4/netfilter/Kconfig
index 690d27d..a355841 100644
--- a/net/ipv4/netfilter/Kconfig
+++ b/net/ipv4/netfilter/Kconfig
@@ -75,6 +75,7 @@ endif # NF_TABLES
 
 config NF_DUP_IPV4
 	tristate "Netfilter IPv4 packet duplication to alternate destination"
+	depends on !NF_CONNTRACK || NF_CONNTRACK
 	help
 	  This option enables the nf_dup_ipv4 core, which duplicates an IPv4
 	  packet to be rerouted to another destination.
diff --git a/net/ipv6/netfilter/Kconfig b/net/ipv6/netfilter/Kconfig
index 96833e4..f6a024e 100644
--- a/net/ipv6/netfilter/Kconfig
+++ b/net/ipv6/netfilter/Kconfig
@@ -58,6 +58,7 @@ endif # NF_TABLES
 
 config NF_DUP_IPV6
 	tristate "Netfilter IPv6 packet duplication to alternate destination"
+	depends on !NF_CONNTRACK || NF_CONNTRACK
 	help
 	  This option enables the nf_dup_ipv6 core, which duplicates an IPv6
 	  packet to be rerouted to another destination.
-- 
2.1.4


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

* [PATCH 2/4] netfilter: ipt_rpfilter: remove the nh_scope test in rpfilter_lookup_reverse
  2015-10-19 18:22 [PATCH 0/4] Netfilter fixes for net Pablo Neira Ayuso
  2015-10-19 18:22 ` [PATCH 1/4] netfilter: fix Kconfig dependencies for nf_dup_ipv{4,6} Pablo Neira Ayuso
@ 2015-10-19 18:22 ` Pablo Neira Ayuso
  2015-10-19 18:22 ` [PATCH 3/4] netfilter: sync with packet rx also after removing queue entries Pablo Neira Ayuso
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2015-10-19 18:22 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: lucien <lucien.xin@gmail.com>

--accept-local  option works for res.type == RTN_LOCAL, which should be
from the local table, but there, the fib_info's nh->nh_scope =
RT_SCOPE_NOWHERE ( > RT_SCOPE_HOST). in fib_create_info().

	if (cfg->fc_scope == RT_SCOPE_HOST) {
		struct fib_nh *nh = fi->fib_nh;

		/* Local address is added. */
		if (nhs != 1 || nh->nh_gw)
			goto err_inval;
		nh->nh_scope = RT_SCOPE_NOWHERE;   <===
		nh->nh_dev = dev_get_by_index(net, fi->fib_nh->nh_oif);
		err = -ENODEV;
		if (!nh->nh_dev)
			goto failure;

but in our rpfilter_lookup_reverse():

	if (dev_match || flags & XT_RPFILTER_LOOSE)
		return FIB_RES_NH(res).nh_scope <= RT_SCOPE_HOST;

if nh->nh_scope > RT_SCOPE_HOST, it will fail. --accept-local option
will never be passed.

it seems the test is bogus and can be removed to fix this issue.

	if (dev_match || flags & XT_RPFILTER_LOOSE)
		return FIB_RES_NH(res).nh_scope <= RT_SCOPE_HOST;

ipv6 does not have this issue.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/ipv4/netfilter/ipt_rpfilter.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/net/ipv4/netfilter/ipt_rpfilter.c b/net/ipv4/netfilter/ipt_rpfilter.c
index 8618fd1..c4ffc9d 100644
--- a/net/ipv4/netfilter/ipt_rpfilter.c
+++ b/net/ipv4/netfilter/ipt_rpfilter.c
@@ -61,9 +61,7 @@ static bool rpfilter_lookup_reverse(struct flowi4 *fl4,
 	if (FIB_RES_DEV(res) == dev)
 		dev_match = true;
 #endif
-	if (dev_match || flags & XT_RPFILTER_LOOSE)
-		return FIB_RES_NH(res).nh_scope <= RT_SCOPE_HOST;
-	return dev_match;
+	return dev_match || flags & XT_RPFILTER_LOOSE;
 }
 
 static bool rpfilter_is_local(const struct sk_buff *skb)
-- 
2.1.4

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

* [PATCH 3/4] netfilter: sync with packet rx also after removing queue entries
  2015-10-19 18:22 [PATCH 0/4] Netfilter fixes for net Pablo Neira Ayuso
  2015-10-19 18:22 ` [PATCH 1/4] netfilter: fix Kconfig dependencies for nf_dup_ipv{4,6} Pablo Neira Ayuso
  2015-10-19 18:22 ` [PATCH 2/4] netfilter: ipt_rpfilter: remove the nh_scope test in rpfilter_lookup_reverse Pablo Neira Ayuso
@ 2015-10-19 18:22 ` Pablo Neira Ayuso
  2015-10-19 18:22 ` [PATCH 4/4] netfilter: ipset: Fix sleeping memory allocation in atomic context Pablo Neira Ayuso
  2015-10-22  2:27 ` [PATCH 0/4] Netfilter fixes for net David Miller
  4 siblings, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2015-10-19 18:22 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Florian Westphal <fw@strlen.de>

We need to sync packet rx again after flushing the queue entries.
Otherwise, the following race could happen:

cpu1: nf_unregister_hook(H) called, H unliked from lists, calls
synchronize_net() to wait for packet rx completion.

Problem is that while no new nf_queue_entry structs that use H can be
allocated, another CPU might receive a verdict from userspace just before
cpu1 calls nf_queue_nf_hook_drop to remove this entry:

cpu2: receive verdict from userspace, lock queue
cpu2: unlink nf_queue_entry struct E, which references H, from queue list
cpu1: calls nf_queue_nf_hook_drop, blocks on queue spinlock
cpu2: unlock queue
cpu1: nf_queue_nf_hook_drop drops affected queue entries
cpu2: call nf_reinject for E
cpu1: kfree(H)
cpu2: potential use-after-free for H

Cc: Eric W. Biederman <ebiederm@xmission.com>
Fixes: 085db2c04557 ("netfilter: Per network namespace netfilter hooks.")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 8e47f81..21a0856 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -152,6 +152,8 @@ void nf_unregister_net_hook(struct net *net, const struct nf_hook_ops *reg)
 #endif
 	synchronize_net();
 	nf_queue_nf_hook_drop(net, &entry->ops);
+	/* other cpu might still process nfqueue verdict that used reg */
+	synchronize_net();
 	kfree(entry);
 }
 EXPORT_SYMBOL(nf_unregister_net_hook);
-- 
2.1.4

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

* [PATCH 4/4] netfilter: ipset: Fix sleeping memory allocation in atomic context
  2015-10-19 18:22 [PATCH 0/4] Netfilter fixes for net Pablo Neira Ayuso
                   ` (2 preceding siblings ...)
  2015-10-19 18:22 ` [PATCH 3/4] netfilter: sync with packet rx also after removing queue entries Pablo Neira Ayuso
@ 2015-10-19 18:22 ` Pablo Neira Ayuso
  2015-10-22  2:27 ` [PATCH 0/4] Netfilter fixes for net David Miller
  4 siblings, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2015-10-19 18:22 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Nikolay Borisov <kernel@kyup.com>

Commit 00590fdd5be0 introduced RCU locking in list type and in
doing so introduced a memory allocation in list_set_add, which
is done in an atomic context, due to the fact that ipset rcu
list modifications are serialised with a spin lock. The reason
why we can't use a mutex is that in addition to modifying the
list with ipset commands, it's also being modified when a
particular ipset rule timeout expires aka garbage collection.
This gc is triggered from set_cleanup_entries, which in turn
is invoked from a timer thus requiring the lock to be bh-safe.

Concretely the following call chain can lead to "sleeping function
called in atomic context" splat:
call_ad -> list_set_uadt -> list_set_uadd -> kzalloc(, GFP_KERNEL).
And since GFP_KERNEL allows initiating direct reclaim thus
potentially sleeping in the allocation path.

To fix the issue change the allocation type to GFP_ATOMIC, to
correctly reflect that it is occuring in an atomic context.

Fixes: 00590fdd5be0 ("netfilter: ipset: Introduce RCU locking in list type")
Signed-off-by: Nikolay Borisov <kernel@kyup.com>
Acked-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/ipset/ip_set_list_set.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/ipset/ip_set_list_set.c b/net/netfilter/ipset/ip_set_list_set.c
index a1fe537..5a30ce6 100644
--- a/net/netfilter/ipset/ip_set_list_set.c
+++ b/net/netfilter/ipset/ip_set_list_set.c
@@ -297,7 +297,7 @@ list_set_uadd(struct ip_set *set, void *value, const struct ip_set_ext *ext,
 	      ip_set_timeout_expired(ext_timeout(n, set))))
 		n =  NULL;
 
-	e = kzalloc(set->dsize, GFP_KERNEL);
+	e = kzalloc(set->dsize, GFP_ATOMIC);
 	if (!e)
 		return -ENOMEM;
 	e->id = d->id;
-- 
2.1.4


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

* Re: [PATCH 0/4] Netfilter fixes for net
  2015-10-19 18:22 [PATCH 0/4] Netfilter fixes for net Pablo Neira Ayuso
                   ` (3 preceding siblings ...)
  2015-10-19 18:22 ` [PATCH 4/4] netfilter: ipset: Fix sleeping memory allocation in atomic context Pablo Neira Ayuso
@ 2015-10-22  2:27 ` David Miller
  4 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2015-10-22  2:27 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Mon, 19 Oct 2015 20:22:51 +0200

> The following patchset contains four Netfilter fixes for net, they are:
> 
> 1) Fix Kconfig dependencies of new nf_dup_ipv4 and nf_dup_ipv6.
> 
> 2) Remove bogus test nh_scope in IPv4 rpfilter match that is breaking
>    --accept-local, from Xin Long.
> 
> 3) Wait for RCU grace period after dropping the pending packets in the
>    nfqueue, from Florian Westphal.
> 
> 4) Fix sleeping allocation while holding spin_lock_bh, from Nikolay Borisov.
> 
> You can pull these changes from:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git

Pulled, thanks a lot Pablo.

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

end of thread, other threads:[~2015-10-22  2:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-19 18:22 [PATCH 0/4] Netfilter fixes for net Pablo Neira Ayuso
2015-10-19 18:22 ` [PATCH 1/4] netfilter: fix Kconfig dependencies for nf_dup_ipv{4,6} Pablo Neira Ayuso
2015-10-19 18:22 ` [PATCH 2/4] netfilter: ipt_rpfilter: remove the nh_scope test in rpfilter_lookup_reverse Pablo Neira Ayuso
2015-10-19 18:22 ` [PATCH 3/4] netfilter: sync with packet rx also after removing queue entries Pablo Neira Ayuso
2015-10-19 18:22 ` [PATCH 4/4] netfilter: ipset: Fix sleeping memory allocation in atomic context Pablo Neira Ayuso
2015-10-22  2:27 ` [PATCH 0/4] Netfilter fixes for net David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).