[Xenial][PATCH 0/2] Fix for Creating conntrack entry failure with kernel 4.4.0-89

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[Xenial][PATCH 0/2] Fix for Creating conntrack entry failure with kernel 4.4.0-89

Kleber Souza
BugLink: http://bugs.launchpad.net/bugs/1709032

We did a re-spin from the last SRU cycle to revert "netfilter: synproxy:
fix conntrackd interaction", which was causing a regression with OpenStack.
A follow-up fix (9c3f3794 - "netfilter: nf_ct_ext: fix possible panic after
nf_ct_extend_unregister") has been found to fix the issue as reported on
the bug.

This patch series re-introduces the reverted patch and adds a cherry pick
from the proper fix.

Kleber Sacilotto de Souza (1):
  Revert "Revert "netfilter: synproxy: fix conntrackd interaction""

Liping Zhang (1):
  netfilter: nf_ct_ext: fix possible panic after nf_ct_extend_unregister

 net/netfilter/nf_conntrack_extend.c  | 13 ++++++++++---
 net/netfilter/nf_conntrack_netlink.c |  4 ++++
 2 files changed, 14 insertions(+), 3 deletions(-)

--
2.14.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[Xenial][PATCH 1/2] Revert "Revert "netfilter: synproxy: fix conntrackd interaction""

Kleber Souza
BugLink: http://bugs.launchpad.net/bugs/1709032

This reverts commit f58e6473b4e026aa2dbd14f008a16b19a0359ad6.
Signed-off-by: Kleber Sacilotto de Souza <[hidden email]>
---
 net/netfilter/nf_conntrack_netlink.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 9f5272968abb..e565b2becb14 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -45,6 +45,8 @@
 #include <net/netfilter/nf_conntrack_zones.h>
 #include <net/netfilter/nf_conntrack_timestamp.h>
 #include <net/netfilter/nf_conntrack_labels.h>
+#include <net/netfilter/nf_conntrack_seqadj.h>
+#include <net/netfilter/nf_conntrack_synproxy.h>
 #ifdef CONFIG_NF_NAT_NEEDED
 #include <net/netfilter/nf_nat_core.h>
 #include <net/netfilter/nf_nat_l4proto.h>
@@ -1798,6 +1800,8 @@ ctnetlink_create_conntrack(struct net *net,
  nf_ct_tstamp_ext_add(ct, GFP_ATOMIC);
  nf_ct_ecache_ext_add(ct, 0, 0, GFP_ATOMIC);
  nf_ct_labels_ext_add(ct);
+ nfct_seqadj_ext_add(ct);
+ nfct_synproxy_ext_add(ct);
 
  /* we must add conntrack extensions before confirmation. */
  ct->status |= IPS_CONFIRMED;
--
2.14.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[Xenial][PATCH 2/2] netfilter: nf_ct_ext: fix possible panic after nf_ct_extend_unregister

Kleber Souza
In reply to this post by Kleber Souza
From: Liping Zhang <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1709032

If one cpu is doing nf_ct_extend_unregister while another cpu is doing
__nf_ct_ext_add_length, then we may hit BUG_ON(t == NULL). Moreover,
there's no synchronize_rcu invocation after set nf_ct_ext_types[id] to
NULL, so it's possible that we may access invalid pointer.

But actually, most of the ct extends are built-in, so the problem listed
above will not happen. However, there are two exceptions: NF_CT_EXT_NAT
and NF_CT_EXT_SYNPROXY.

For _EXT_NAT, the panic will not happen, since adding the nat extend and
unregistering the nat extend are located in the same file(nf_nat_core.c),
this means that after the nat module is removed, we cannot add the nat
extend too.

For _EXT_SYNPROXY, synproxy extend may be added by init_conntrack, while
synproxy extend unregister will be done by synproxy_core_exit. So after
nf_synproxy_core.ko is removed, we may still try to add the synproxy
extend, then kernel panic may happen.

I know it's very hard to reproduce this issue, but I can play a tricky
game to make it happen very easily :)

Step 1. Enable SYNPROXY for tcp dport 1234 at FORWARD hook:
  # iptables -I FORWARD -p tcp --dport 1234 -j SYNPROXY
Step 2. Queue the syn packet to the userspace at raw table OUTPUT hook.
        Also note, in the userspace we only add a 20s' delay, then
        reinject the syn packet to the kernel:
  # iptables -t raw -I OUTPUT -p tcp --syn -j NFQUEUE --queue-num 1
Step 3. Using "nc 2.2.2.2 1234" to connect the server.
Step 4. Now remove the nf_synproxy_core.ko quickly:
  # iptables -F FORWARD
  # rmmod ipt_SYNPROXY
  # rmmod nf_synproxy_core
Step 5. After 20s' delay, the syn packet is reinjected to the kernel.

Now you will see the panic like this:
  kernel BUG at net/netfilter/nf_conntrack_extend.c:91!
  Call Trace:
   ? __nf_ct_ext_add_length+0x53/0x3c0 [nf_conntrack]
   init_conntrack+0x12b/0x600 [nf_conntrack]
   nf_conntrack_in+0x4cc/0x580 [nf_conntrack]
   ipv4_conntrack_local+0x48/0x50 [nf_conntrack_ipv4]
   nf_reinject+0x104/0x270
   nfqnl_recv_verdict+0x3e1/0x5f9 [nfnetlink_queue]
   ? nfqnl_recv_verdict+0x5/0x5f9 [nfnetlink_queue]
   ? nla_parse+0xa0/0x100
   nfnetlink_rcv_msg+0x175/0x6a9 [nfnetlink]
   [...]

One possible solution is to make NF_CT_EXT_SYNPROXY extend built-in, i.e.
introduce nf_conntrack_synproxy.c and only do ct extend register and
unregister in it, similar to nf_conntrack_timeout.c.

But having such a obscure restriction of nf_ct_extend_unregister is not a
good idea, so we should invoke synchronize_rcu after set nf_ct_ext_types
to NULL, and check the NULL pointer when do __nf_ct_ext_add_length. Then
it will be easier if we add new ct extend in the future.

Last, we use kfree_rcu to free nf_ct_ext, so rcu_barrier() is unnecessary
anymore, remove it too.

Signed-off-by: Liping Zhang <[hidden email]>
Acked-by: Florian Westphal <[hidden email]>
Signed-off-by: Pablo Neira Ayuso <[hidden email]>
(cherry picked from commit 9c3f3794926a997b1cab6c42480ff300efa2d162)
Signed-off-by: Kleber Sacilotto de Souza <[hidden email]>
---
 net/netfilter/nf_conntrack_extend.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/nf_conntrack_extend.c b/net/netfilter/nf_conntrack_extend.c
index 1a9545965c0d..531ca55f1af6 100644
--- a/net/netfilter/nf_conntrack_extend.c
+++ b/net/netfilter/nf_conntrack_extend.c
@@ -53,7 +53,11 @@ nf_ct_ext_create(struct nf_ct_ext **ext, enum nf_ct_ext_id id,
 
  rcu_read_lock();
  t = rcu_dereference(nf_ct_ext_types[id]);
- BUG_ON(t == NULL);
+ if (!t) {
+ rcu_read_unlock();
+ return NULL;
+ }
+
  off = ALIGN(sizeof(struct nf_ct_ext), t->align);
  len = off + t->len + var_alloc_len;
  alloc_size = t->alloc_size + var_alloc_len;
@@ -88,7 +92,10 @@ void *__nf_ct_ext_add_length(struct nf_conn *ct, enum nf_ct_ext_id id,
 
  rcu_read_lock();
  t = rcu_dereference(nf_ct_ext_types[id]);
- BUG_ON(t == NULL);
+ if (!t) {
+ rcu_read_unlock();
+ return NULL;
+ }
 
  newoff = ALIGN(old->len, t->align);
  newlen = newoff + t->len + var_alloc_len;
@@ -186,6 +193,6 @@ void nf_ct_extend_unregister(struct nf_ct_ext_type *type)
  RCU_INIT_POINTER(nf_ct_ext_types[type->id], NULL);
  update_alloc_size(type);
  mutex_unlock(&nf_ct_ext_type_mutex);
- rcu_barrier(); /* Wait for completion of call_rcu()'s */
+ synchronize_rcu();
 }
 EXPORT_SYMBOL_GPL(nf_ct_extend_unregister);
--
2.14.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

ACK: [Xenial][PATCH 0/2] Fix for Creating conntrack entry failure with kernel 4.4.0-89

Stefan Bader-2
In reply to this post by Kleber Souza
On 11.08.2017 15:28, Kleber Sacilotto de Souza wrote:

> BugLink: http://bugs.launchpad.net/bugs/1709032
>
> We did a re-spin from the last SRU cycle to revert "netfilter: synproxy:
> fix conntrackd interaction", which was causing a regression with OpenStack.
> A follow-up fix (9c3f3794 - "netfilter: nf_ct_ext: fix possible panic after
> nf_ct_extend_unregister") has been found to fix the issue as reported on
> the bug.
>
> This patch series re-introduces the reverted patch and adds a cherry pick
> from the proper fix.
>
> Kleber Sacilotto de Souza (1):
>   Revert "Revert "netfilter: synproxy: fix conntrackd interaction""
>
> Liping Zhang (1):
>   netfilter: nf_ct_ext: fix possible panic after nf_ct_extend_unregister
>
>  net/netfilter/nf_conntrack_extend.c  | 13 ++++++++++---
>  net/netfilter/nf_conntrack_netlink.c |  4 ++++
>  2 files changed, 14 insertions(+), 3 deletions(-)
>
Acked-by: Stefan Bader <[hidden email]>

Sounds like the better way fwd, lets make sure the affected parties from the
regression make some testing with that.

-Stefan



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

APPLIED: [Xenial][PATCH 0/2] Fix for Creating conntrack entry failure with kernel 4.4.0-89

Kleber Souza
In reply to this post by Kleber Souza
Applied on xenial/master-next branch.

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Loading...