[net PATCH 0/2] Fixing OOPSes in seqadj code

[net PATCH 0/2] Fixing OOPSes in seqadj code

[Jesper Dangaard Brouer]

The following series address issues with netfilter conntrack sequence
number adjustments, introduced with commit 41d73ec053d2 (netfilter:
nf_conntrack: make sequence number adjustments usuable without NAT).

 Patch1: give us a WARN splash when using seqadj incorrectly

 Patch2: fixes a wrong usage of seqadj in IPVS code

I'm not sure, which maintainer will take these fixes?!?

---

Jesper Dangaard Brouer (2):
      ipvs: correct usage/allocation of seqadj ext in ipvs
      netfilter: WARN about wrong usage of sequence number adjustments


 net/netfilter/ipvs/ip_vs_nfct.c     |    6 ++++++
 net/netfilter/nf_conntrack_seqadj.c |    5 +++++
 2 files changed, 11 insertions(+), 0 deletions(-)

[net PATCH 1/2] netfilter: WARN about wrong usage of sequence number adjustments

[Jesper Dangaard Brouer]

Since commit 41d73ec053d2 (netfilter: nf_conntrack: make sequence
number adjustments usuable without NAT), the sequence number extension
is dynamically allocated.

Instead of dying, give a WARN splash, in case of wrong usage of the
seqadj code, e.g. when forgetting to allocate via nfct_seqadj_ext_add().

Wrong usage have been seen in the IPVS code path.

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---

 net/netfilter/nf_conntrack_seqadj.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/net/netfilter/nf_conntrack_seqadj.c b/net/netfilter/nf_conntrack_seqadj.c
index 17c1bcb..b2d38da 100644
--- a/net/netfilter/nf_conntrack_seqadj.c
+++ b/net/netfilter/nf_conntrack_seqadj.c
@@ -36,6 +36,11 @@ int nf_ct_seqadj_set(struct nf_conn *ct, enum ip_conntrack_info ctinfo,
 	if (off == 0)
 		return 0;
 
+	if (unlikely(!seqadj)) {
+		WARN(1, "Wrong seqadj usage, missing nfct_seqadj_ext_add()\n");
+		return 0;
+	}
+
 	set_bit(IPS_SEQ_ADJUST_BIT, &ct->status);
 
 	spin_lock_bh(&ct->lock);

[net PATCH 2/2] ipvs: correct usage/allocation of seqadj ext in ipvs

[Jesper Dangaard Brouer]

The IPVS FTP helper ip_vs_ftp could trigger an OOPS in nf_ct_seqadj_set,
after commit 41d73ec053d2 (netfilter: nf_conntrack: make sequence number
adjustments usuable without NAT).

This is because, the seqadj ext is now allocated dynamically, and the
IPVS code didn't handle this situation.  Fix this in the IPVS nfct
code by invoking the alloc function nfct_seqadj_ext_add().

Fixes: 41d73ec053d2 (netfilter: nf_conntrack: make sequence number adjustments usuable without NAT)
Suggested-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---

 net/netfilter/ipvs/ip_vs_nfct.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_nfct.c b/net/netfilter/ipvs/ip_vs_nfct.c
index d5f4151..5882bbf 100644
--- a/net/netfilter/ipvs/ip_vs_nfct.c
+++ b/net/netfilter/ipvs/ip_vs_nfct.c
@@ -62,6 +62,7 @@
 #include <net/ip_vs.h>
 #include <net/netfilter/nf_conntrack_core.h>
 #include <net/netfilter/nf_conntrack_expect.h>
+#include <net/netfilter/nf_conntrack_seqadj.h>
 #include <net/netfilter/nf_conntrack_helper.h>
 #include <net/netfilter/nf_conntrack_zones.h>
 
@@ -96,6 +97,11 @@ ip_vs_update_conntrack(struct sk_buff *skb, struct ip_vs_conn *cp, int outin)
 	if (CTINFO2DIR(ctinfo) != IP_CT_DIR_ORIGINAL)
 		return;
 
+	/* Applications may adjust TCP seqs */
+	if (cp->app && nf_ct_protonum(ct) == IPPROTO_TCP &&
+	    !nfct_seqadj(ct) && !nfct_seqadj_ext_add(ct))
+		return;
+
 	/*
 	 * The connection is not yet in the hashtable, so we update it.
 	 * CIP->VIP will remain the same, so leave the tuple in

Re: [net PATCH 0/2] Fixing OOPSes in seqadj code

[Julian Anastasov]

	Hello,

On Mon, 16 Dec 2013, Jesper Dangaard Brouer wrote:

> The following series address issues with netfilter conntrack sequence
> number adjustments, introduced with commit 41d73ec053d2 (netfilter:
> nf_conntrack: make sequence number adjustments usuable without NAT).
> 
>  Patch1: give us a WARN splash when using seqadj incorrectly
> 
>  Patch2: fixes a wrong usage of seqadj in IPVS code

	Both patches look good to me,

Acked-by: Julian Anastasov <ja@ssi.bg>

> I'm not sure, which maintainer will take these fixes?!?

	I think, Simon can take them, if there are no
objections...

> Jesper Dangaard Brouer (2):
>       ipvs: correct usage/allocation of seqadj ext in ipvs
>       netfilter: WARN about wrong usage of sequence number adjustments
> 
> 
>  net/netfilter/ipvs/ip_vs_nfct.c     |    6 ++++++
>  net/netfilter/nf_conntrack_seqadj.c |    5 +++++
>  2 files changed, 11 insertions(+), 0 deletions(-)

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [net PATCH 0/2] Fixing OOPSes in seqadj code

[Simon Horman]

On Mon, Dec 16, 2013 at 11:11:51PM +0200, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Mon, 16 Dec 2013, Jesper Dangaard Brouer wrote:
> 
> > The following series address issues with netfilter conntrack sequence
> > number adjustments, introduced with commit 41d73ec053d2 (netfilter:
> > nf_conntrack: make sequence number adjustments usuable without NAT).
> > 
> >  Patch1: give us a WARN splash when using seqadj incorrectly
> > 
> >  Patch2: fixes a wrong usage of seqadj in IPVS code
> 
> 	Both patches look good to me,
> 
> Acked-by: Julian Anastasov <ja@ssi.bg>
> 
> > I'm not sure, which maintainer will take these fixes?!?
> 
> 	I think, Simon can take them, if there are no
> objections...

I have no objections.
I'll wait a day to see if anyone else does.

> 
> > Jesper Dangaard Brouer (2):
> >       ipvs: correct usage/allocation of seqadj ext in ipvs
> >       netfilter: WARN about wrong usage of sequence number adjustments
> > 
> > 
> >  net/netfilter/ipvs/ip_vs_nfct.c     |    6 ++++++
> >  net/netfilter/nf_conntrack_seqadj.c |    5 +++++
> >  2 files changed, 11 insertions(+), 0 deletions(-)
> 
> Regards
> 
> --
> Julian Anastasov <ja@ssi.bg>
> 

Re: [net PATCH 0/2] Fixing OOPSes in seqadj code

[Simon Horman]

On Tue, Dec 17, 2013 at 10:36:37AM +0900, Simon Horman wrote:
> On Mon, Dec 16, 2013 at 11:11:51PM +0200, Julian Anastasov wrote:
> > 
> > 	Hello,
> > 
> > On Mon, 16 Dec 2013, Jesper Dangaard Brouer wrote:
> > 
> > > The following series address issues with netfilter conntrack sequence
> > > number adjustments, introduced with commit 41d73ec053d2 (netfilter:
> > > nf_conntrack: make sequence number adjustments usuable without NAT).
> > > 
> > >  Patch1: give us a WARN splash when using seqadj incorrectly
> > > 
> > >  Patch2: fixes a wrong usage of seqadj in IPVS code
> > 
> > 	Both patches look good to me,
> > 
> > Acked-by: Julian Anastasov <ja@ssi.bg>
> > 
> > > I'm not sure, which maintainer will take these fixes?!?
> > 
> > 	I think, Simon can take them, if there are no
> > objections...
> 
> I have no objections.
> I'll wait a day to see if anyone else does.

It was much more than a day but I have taken these changes now.