[NETFILTER]: ip_conntrack_expect_related must not free expectation

If a connection tracking helper tells us to expect a connection, and
we're already expecting that connection, we simply free the one they
gave us and return success.

The problem is that NAT helpers (eg. FTP) have to allocate the
expectation first (to see what port is available) then rewrite the
packet.  If that rewrite fails, they try to remove the expectation,
but it was freed in ip_conntrack_expect_related.

This is one example of a larger problem: having registered the
expectation, the pointer is no longer ours to use.  Reference counting
is needed for ctnetlink anyway, so introduce it now.

To have a single "put" path, we need to grab the reference to the
connection on creation, rather than open-coding it in the caller.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/include/linux/netfilter_ipv4/ip_conntrack.h b/include/linux/netfilter_ipv4/ip_conntrack.h
index 3781192ce15944122692dbaca763efaf2a595540..f8da7ddeff3a9dcdb7cba77f7eaad74afb5445a3 100644 (file)
--- a/include/linux/netfilter_ipv4/ip_conntrack.h
+++ b/include/linux/netfilter_ipv4/ip_conntrack.h
@@ -197,6 +197,9 @@ struct ip_conntrack_expect
        /* Timer function; deletes the expectation. */
        struct timer_list timeout;
 
+       /* Usage count. */
+       atomic_t use;
+
 #ifdef CONFIG_IP_NF_NAT_NEEDED
        /* This is the original per-proto part, used to map the
         * expected connection the way the recipient expects. */

diff --git a/include/linux/netfilter_ipv4/ip_conntrack_helper.h b/include/linux/netfilter_ipv4/ip_conntrack_helper.h
index b1bbba0a12cb22343d5f69430e7dde86107d3c2c..3692daa93decf3455368fc3bec5dec91fa1c40f0 100644 (file)
--- a/include/linux/netfilter_ipv4/ip_conntrack_helper.h
+++ b/include/linux/netfilter_ipv4/ip_conntrack_helper.h
@@ -30,9 +30,10 @@ extern int ip_conntrack_helper_register(struct ip_conntrack_helper *);
 extern void ip_conntrack_helper_unregister(struct ip_conntrack_helper *);
 
 /* Allocate space for an expectation: this is mandatory before calling 
-   ip_conntrack_expect_related. */
-extern struct ip_conntrack_expect *ip_conntrack_expect_alloc(void);
-extern void ip_conntrack_expect_free(struct ip_conntrack_expect *exp);
+   ip_conntrack_expect_related.  You will have to call put afterwards. */
+extern struct ip_conntrack_expect *
+ip_conntrack_expect_alloc(struct ip_conntrack *master);
+extern void ip_conntrack_expect_put(struct ip_conntrack_expect *exp);
 
 /* Add an expected connection: can have more than one per connection */
 extern int ip_conntrack_expect_related(struct ip_conntrack_expect *exp);

diff --git a/net/ipv4/netfilter/ip_conntrack_amanda.c b/net/ipv4/netfilter/ip_conntrack_amanda.c
index a78a320eee082802eebb99d0b468eda935160934..01e1b58322a9b07cba23e4d2b678debf873c42d1 100644 (file)
--- a/net/ipv4/netfilter/ip_conntrack_amanda.c
+++ b/net/ipv4/netfilter/ip_conntrack_amanda.c
@@ -101,14 +101,13 @@ static int help(struct sk_buff **pskb,
                if (port == 0 || len > 5)
                        break;
 
-               exp = ip_conntrack_expect_alloc();
+               exp = ip_conntrack_expect_alloc(ct);
                if (exp == NULL) {
                        ret = NF_DROP;
                        goto out;
                }
 
                exp->expectfn = NULL;
-               exp->master = ct;
 
                exp->tuple.src.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
                exp->tuple.src.u.tcp.port = 0;
@@ -126,10 +125,9 @@ static int help(struct sk_buff **pskb,
                        ret = ip_nat_amanda_hook(pskb, ctinfo,
                                                 tmp - amanda_buffer,
                                                 len, exp);
-               else if (ip_conntrack_expect_related(exp) != 0) {
-                       ip_conntrack_expect_free(exp);
+               else if (ip_conntrack_expect_related(exp) != 0)
                        ret = NF_DROP;
-               }
+               ip_conntrack_expect_put(exp);
        }
 
 out:

diff --git a/net/ipv4/netfilter/ip_conntrack_core.c b/net/ipv4/netfilter/ip_conntrack_core.c
index 4b78ebeb66350e9616bf03d6468fc039b53716e6..14af55cad5d6af3130a8f89fdfa1f2506ea62056 100644 (file)
--- a/net/ipv4/netfilter/ip_conntrack_core.c
+++ b/net/ipv4/netfilter/ip_conntrack_core.c
@@ -137,19 +137,12 @@ ip_ct_invert_tuple(struct ip_conntrack_tuple *inverse,
 
 
 /* ip_conntrack_expect helper functions */
-static void destroy_expect(struct ip_conntrack_expect *exp)
-{
-       ip_conntrack_put(exp->master);
-       IP_NF_ASSERT(!timer_pending(&exp->timeout));
-       kmem_cache_free(ip_conntrack_expect_cachep, exp);
-       CONNTRACK_STAT_INC(expect_delete);
-}
-
 static void unlink_expect(struct ip_conntrack_expect *exp)
 {
        ASSERT_WRITE_LOCK(&ip_conntrack_lock);
+       IP_NF_ASSERT(!timer_pending(&exp->timeout));
        list_del(&exp->list);
-       /* Logically in destroy_expect, but we hold the lock here. */
+       CONNTRACK_STAT_INC(expect_delete);
        exp->master->expecting--;
 }
 
@@ -160,7 +153,7 @@ static void expectation_timed_out(unsigned long ul_expect)
        write_lock_bh(&ip_conntrack_lock);
        unlink_expect(exp);
        write_unlock_bh(&ip_conntrack_lock);
-       destroy_expect(exp);
+       ip_conntrack_expect_put(exp);
 }
 
 /* If an expectation for this connection is found, it gets delete from
@@ -198,7 +191,7 @@ static void remove_expectations(struct ip_conntrack *ct)
        list_for_each_entry_safe(i, tmp, &ip_conntrack_expect_list, list) {
                if (i->master == ct && del_timer(&i->timeout)) {
                        unlink_expect(i);
-                       destroy_expect(i);
+                       ip_conntrack_expect_put(i);
                }
        }
 }
@@ -537,7 +530,7 @@ init_conntrack(const struct ip_conntrack_tuple *tuple,
        if (exp) {
                if (exp->expectfn)
                        exp->expectfn(conntrack, exp);
-               destroy_expect(exp);
+               ip_conntrack_expect_put(exp);
        }
 
        return &conntrack->tuplehash[IP_CT_DIR_ORIGINAL];
@@ -729,14 +722,14 @@ void ip_conntrack_unexpect_related(struct ip_conntrack_expect *exp)
                if (expect_matches(i, exp) && del_timer(&i->timeout)) {
                        unlink_expect(i);
                        write_unlock_bh(&ip_conntrack_lock);
-                       destroy_expect(i);
+                       ip_conntrack_expect_put(i);
                        return;
                }
        }
        write_unlock_bh(&ip_conntrack_lock);
 }
 
-struct ip_conntrack_expect *ip_conntrack_expect_alloc(void)
+struct ip_conntrack_expect *ip_conntrack_expect_alloc(struct ip_conntrack *me)
 {
        struct ip_conntrack_expect *new;
 
@@ -745,18 +738,23 @@ struct ip_conntrack_expect *ip_conntrack_expect_alloc(void)
                DEBUGP("expect_related: OOM allocating expect\n");
                return NULL;
        }
-       new->master = NULL;
+       new->master = me;
+       atomic_inc(&new->master->ct_general.use);
+       atomic_set(&new->use, 1);
        return new;
 }
 
-void ip_conntrack_expect_free(struct ip_conntrack_expect *expect)
+void ip_conntrack_expect_put(struct ip_conntrack_expect *exp)
 {
-       kmem_cache_free(ip_conntrack_expect_cachep, expect);
+       if (atomic_dec_and_test(&exp->use)) {
+               ip_conntrack_put(exp->master);
+               kmem_cache_free(ip_conntrack_expect_cachep, exp);
+       }
 }
 
 static void ip_conntrack_expect_insert(struct ip_conntrack_expect *exp)
 {
-       atomic_inc(&exp->master->ct_general.use);
+       atomic_inc(&exp->use);
        exp->master->expecting++;
        list_add(&exp->list, &ip_conntrack_expect_list);
 
@@ -778,7 +776,7 @@ static void evict_oldest_expect(struct ip_conntrack *master)
                if (i->master == master) {
                        if (del_timer(&i->timeout)) {
                                unlink_expect(i);
-                               destroy_expect(i);
+                               ip_conntrack_expect_put(i);
                        }
                        break;
                }
@@ -810,8 +808,6 @@ int ip_conntrack_expect_related(struct ip_conntrack_expect *expect)
                        /* Refresh timer: if it's dying, ignore.. */
                        if (refresh_timer(i)) {
                                ret = 0;
-                               /* We don't need the one they've given us. */
-                               ip_conntrack_expect_free(expect);
                                goto out;
                        }
                } else if (expect_clash(i, expect)) {
@@ -881,7 +877,7 @@ void ip_conntrack_helper_unregister(struct ip_conntrack_helper *me)
        list_for_each_entry_safe(exp, tmp, &ip_conntrack_expect_list, list) {
                if (exp->master->helper == me && del_timer(&exp->timeout)) {
                        unlink_expect(exp);
-                       destroy_expect(exp);
+                       ip_conntrack_expect_put(exp);
                }
        }
        /* Get rid of expecteds, set helpers to NULL. */

diff --git a/net/ipv4/netfilter/ip_conntrack_ftp.c b/net/ipv4/netfilter/ip_conntrack_ftp.c
index fea6dd2a00b6b2db30e2ab348f02b354ff40d443..7a3b773be3f93cf4e7a2d1d706fec6d63f3f252c 100644 (file)
--- a/net/ipv4/netfilter/ip_conntrack_ftp.c
+++ b/net/ipv4/netfilter/ip_conntrack_ftp.c
@@ -376,7 +376,7 @@ static int help(struct sk_buff **pskb,
               fb_ptr + matchoff, matchlen, ntohl(th->seq) + matchoff);
                         
        /* Allocate expectation which will be inserted */
-       exp = ip_conntrack_expect_alloc();
+       exp = ip_conntrack_expect_alloc(ct);
        if (exp == NULL) {
                ret = NF_DROP;
                goto out;
@@ -403,8 +403,7 @@ static int help(struct sk_buff **pskb,
                   networks, or the packet filter itself). */
                if (!loose) {
                        ret = NF_ACCEPT;
-                       ip_conntrack_expect_free(exp);
-                       goto out_update_nl;
+                       goto out_put_expect;
                }
                exp->tuple.dst.ip = htonl((array[0] << 24) | (array[1] << 16)
                                         | (array[2] << 8) | array[3]);
@@ -419,7 +418,6 @@ static int help(struct sk_buff **pskb,
                  { 0xFFFFFFFF, { .tcp = { 0xFFFF } }, 0xFF }});
 
        exp->expectfn = NULL;
-       exp->master = ct;
 
        /* Now, NAT might want to mangle the packet, and register the
         * (possibly changed) expectation itself. */
@@ -428,13 +426,15 @@ static int help(struct sk_buff **pskb,
                                      matchoff, matchlen, exp, &seq);
        else {
                /* Can't expect this?  Best to drop packet now. */
-               if (ip_conntrack_expect_related(exp) != 0) {
-                       ip_conntrack_expect_free(exp);
+               if (ip_conntrack_expect_related(exp) != 0)
                        ret = NF_DROP;
-               } else
+               else
                        ret = NF_ACCEPT;
        }
 
+out_put_expect:
+       ip_conntrack_expect_put(exp);
+
 out_update_nl:
        /* Now if this ends in \n, update ftp info.  Seq may have been
         * adjusted by NAT code. */

diff --git a/net/ipv4/netfilter/ip_conntrack_irc.c b/net/ipv4/netfilter/ip_conntrack_irc.c
index cd98772cc332a2bf3ac5ea6b91c5df9a60f06355..4a28f297d502a38d4c10b780306b5bfca127f97f 100644 (file)
--- a/net/ipv4/netfilter/ip_conntrack_irc.c
+++ b/net/ipv4/netfilter/ip_conntrack_irc.c
@@ -197,7 +197,7 @@ static int help(struct sk_buff **pskb,
                                continue;
                        }
 
-                       exp = ip_conntrack_expect_alloc();
+                       exp = ip_conntrack_expect_alloc(ct);
                        if (exp == NULL) {
                                ret = NF_DROP;
                                goto out;
@@ -221,16 +221,14 @@ static int help(struct sk_buff **pskb,
                                { { 0, { 0 } },
                                  { 0xFFFFFFFF, { .tcp = { 0xFFFF } }, 0xFF }});
                        exp->expectfn = NULL;
-                       exp->master = ct;
                        if (ip_nat_irc_hook)
                                ret = ip_nat_irc_hook(pskb, ctinfo, 
                                                      addr_beg_p - ib_ptr,
                                                      addr_end_p - addr_beg_p,
                                                      exp);
-                       else if (ip_conntrack_expect_related(exp) != 0) {
-                               ip_conntrack_expect_free(exp);
+                       else if (ip_conntrack_expect_related(exp) != 0)
                                ret = NF_DROP;
-                       }
+                       ip_conntrack_expect_put(exp);
                        goto out;
                } /* for .. NUM_DCCPROTO */
        } /* while data < ... */

diff --git a/net/ipv4/netfilter/ip_conntrack_standalone.c b/net/ipv4/netfilter/ip_conntrack_standalone.c
index 1dd824f3cf0a1db3ed0daf7f961ceac28402953a..61798c46e91d06e051947db664c301a45b5b6b74 100644 (file)
--- a/net/ipv4/netfilter/ip_conntrack_standalone.c
+++ b/net/ipv4/netfilter/ip_conntrack_standalone.c
@@ -985,7 +985,7 @@ EXPORT_SYMBOL(ip_ct_refresh_acct);
 EXPORT_SYMBOL(ip_ct_protos);
 EXPORT_SYMBOL(ip_ct_find_proto);
 EXPORT_SYMBOL(ip_conntrack_expect_alloc);
-EXPORT_SYMBOL(ip_conntrack_expect_free);
+EXPORT_SYMBOL(ip_conntrack_expect_put);
 EXPORT_SYMBOL(ip_conntrack_expect_related);
 EXPORT_SYMBOL(ip_conntrack_unexpect_related);
 EXPORT_SYMBOL(ip_conntrack_tuple_taken);

diff --git a/net/ipv4/netfilter/ip_conntrack_tftp.c b/net/ipv4/netfilter/ip_conntrack_tftp.c
index 992fac3e36eea9edd08fd3bc5052457cb5cc0841..f8ff170f390a3ad8bd8e5f0100658d7d1908008f 100644 (file)
--- a/net/ipv4/netfilter/ip_conntrack_tftp.c
+++ b/net/ipv4/netfilter/ip_conntrack_tftp.c
@@ -65,7 +65,7 @@ static int tftp_help(struct sk_buff **pskb,
                DUMP_TUPLE(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
                DUMP_TUPLE(&ct->tuplehash[IP_CT_DIR_REPLY].tuple);
 
-               exp = ip_conntrack_expect_alloc();
+               exp = ip_conntrack_expect_alloc(ct);
                if (exp == NULL)
                        return NF_DROP;
 
@@ -75,17 +75,15 @@ static int tftp_help(struct sk_buff **pskb,
                exp->mask.dst.u.udp.port = 0xffff;
                exp->mask.dst.protonum = 0xff;
                exp->expectfn = NULL;
-               exp->master = ct;
 
                DEBUGP("expect: ");
                DUMP_TUPLE(&exp->tuple);
                DUMP_TUPLE(&exp->mask);
                if (ip_nat_tftp_hook)
                        ret = ip_nat_tftp_hook(pskb, ctinfo, exp);
-               else if (ip_conntrack_expect_related(exp) != 0) {
-                       ip_conntrack_expect_free(exp);
+               else if (ip_conntrack_expect_related(exp) != 0)
                        ret = NF_DROP;
-               }
+               ip_conntrack_expect_put(exp);
                break;
        case TFTP_OPCODE_DATA:
        case TFTP_OPCODE_ACK:

diff --git a/net/ipv4/netfilter/ip_nat_amanda.c b/net/ipv4/netfilter/ip_nat_amanda.c
index da1f412583edf355da343691b921dec230fc840e..706c8074f42228af2d9d716fd4e4fc6d786a5433 100644 (file)
--- a/net/ipv4/netfilter/ip_nat_amanda.c
+++ b/net/ipv4/netfilter/ip_nat_amanda.c
@@ -56,10 +56,8 @@ static unsigned int help(struct sk_buff **pskb,
                        break;
        }
 
-       if (port == 0) {
-               ip_conntrack_expect_free(exp);
+       if (port == 0)
                return NF_DROP;
-       }
 
        sprintf(buffer, "%u", port);
        ret = ip_nat_mangle_udp_packet(pskb, exp->master, ctinfo,

diff --git a/net/ipv4/netfilter/ip_nat_ftp.c b/net/ipv4/netfilter/ip_nat_ftp.c
index c6000e794ad67ca6089eb7583ae8c81914fbcea6..d83757a70d9f647f2474952c79b424b148d5159a 100644 (file)
--- a/net/ipv4/netfilter/ip_nat_ftp.c
+++ b/net/ipv4/netfilter/ip_nat_ftp.c
@@ -143,10 +143,8 @@ static unsigned int ip_nat_ftp(struct sk_buff **pskb,
                        break;
        }
 
-       if (port == 0) {
-               ip_conntrack_expect_free(exp);
+       if (port == 0)
                return NF_DROP;
-       }
 
        if (!mangle[type](pskb, newip, port, matchoff, matchlen, ct, ctinfo,
                          seq)) {

diff --git a/net/ipv4/netfilter/ip_nat_irc.c b/net/ipv4/netfilter/ip_nat_irc.c
index 9c1ca3381d561fbdd0cae9fae448861421abac36..de31942babe399e3fe9a58919505276662c04bf9 100644 (file)
--- a/net/ipv4/netfilter/ip_nat_irc.c
+++ b/net/ipv4/netfilter/ip_nat_irc.c
@@ -65,10 +65,8 @@ static unsigned int help(struct sk_buff **pskb,
                        break;
        }
 
-       if (port == 0) {
-               ip_conntrack_expect_free(exp);
+       if (port == 0)
                return NF_DROP;
-       }
 
        /*      strlen("\1DCC CHAT chat AAAAAAAA P\1\n")=27
         *      strlen("\1DCC SCHAT chat AAAAAAAA P\1\n")=28

diff --git a/net/ipv4/netfilter/ip_nat_tftp.c b/net/ipv4/netfilter/ip_nat_tftp.c
index 0343e0d64674d46a1b97d017036b65f5dded1772..2215317c76b7d177299969b87adbc31feabc6a51 100644 (file)
--- a/net/ipv4/netfilter/ip_nat_tftp.c
+++ b/net/ipv4/netfilter/ip_nat_tftp.c
@@ -45,10 +45,8 @@ static unsigned int help(struct sk_buff **pskb,
        exp->saved_proto.udp.port = exp->tuple.dst.u.tcp.port;
        exp->dir = IP_CT_DIR_REPLY;
        exp->expectfn = ip_nat_follow_master;
-       if (ip_conntrack_expect_related(exp) != 0) {
-               ip_conntrack_expect_free(exp);
+       if (ip_conntrack_expect_related(exp) != 0)
                return NF_DROP;
-       }
        return NF_ACCEPT;
 }