netfilter: nf_ct_ftp: prefer skb_linearize

This uses a pseudo-linearization scheme with a 64k global buffer,
but BIG TCP arrival means IPv6 TCP stack can generate skbs
that exceed this size.

Use skb_linearize.  It should be possible to rewrite this to properly
deal with segmented skbs (i.e., only do small chunk-wise accesses),
but this is going to be a lot more intrusive than this because every
helper function needs to get the sk_buff instead of a pointer to a raw
data buffer.

In practice, provided we're really looking at FTP control channel packets,
there should never be a case where we deal with huge packets.

Fixes: f4d2f29901ff ("net: allow gso_max_size to exceed 65536")
Fixes: 12478a2acbca ("net: allow gro_max_size to exceed 65536")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/net/netfilter/nf_conntrack_ftp.c b/net/netfilter/nf_conntrack_ftp.c
index a414274338cff03efc477e28f2f43976b9adcd7b..0d9332e9cf71a8fae7a5e7b5a0bc904863c50072 100644 (file)
--- a/net/netfilter/nf_conntrack_ftp.c
+++ b/net/netfilter/nf_conntrack_ftp.c
@@ -34,11 +34,6 @@ MODULE_DESCRIPTION("ftp connection tracking helper");
 MODULE_ALIAS("ip_conntrack_ftp");
 MODULE_ALIAS_NFCT_HELPER(HELPER_NAME);
 
-/* This is slow, but it's simple. --RR */
-static char *ftp_buffer;
-
-static DEFINE_SPINLOCK(nf_ftp_lock);
-
 #define MAX_PORTS 8
 static u_int16_t ports[MAX_PORTS];
 static unsigned int ports_c;
@@ -398,6 +393,9 @@ static int help(struct sk_buff *skb,
                return NF_ACCEPT;
        }
 
+       if (unlikely(skb_linearize(skb)))
+               return NF_DROP;
+
        th = skb_header_pointer(skb, protoff, sizeof(_tcph), &_tcph);
        if (th == NULL)
                return NF_ACCEPT;
@@ -411,12 +409,8 @@ static int help(struct sk_buff *skb,
        }
        datalen = skb->len - dataoff;
 
-       spin_lock_bh(&nf_ftp_lock);
-       fb_ptr = skb_header_pointer(skb, dataoff, datalen, ftp_buffer);
-       if (!fb_ptr) {
-               spin_unlock_bh(&nf_ftp_lock);
-               return NF_ACCEPT;
-       }
+       spin_lock_bh(&ct->lock);
+       fb_ptr = skb->data + dataoff;
 
        ends_in_nl = (fb_ptr[datalen - 1] == '\n');
        seq = ntohl(th->seq) + datalen;
@@ -544,7 +538,7 @@ out_update_nl:
        if (ends_in_nl)
                update_nl_seq(ct, seq, ct_ftp_info, dir, skb);
  out:
-       spin_unlock_bh(&nf_ftp_lock);
+       spin_unlock_bh(&ct->lock);
        return ret;
 }
 
@@ -571,7 +565,6 @@ static const struct nf_conntrack_expect_policy ftp_exp_policy = {
 static void __exit nf_conntrack_ftp_fini(void)
 {
        nf_conntrack_helpers_unregister(ftp, ports_c * 2);
-       kfree(ftp_buffer);
 }
 
 static int __init nf_conntrack_ftp_init(void)
@@ -580,10 +573,6 @@ static int __init nf_conntrack_ftp_init(void)
 
        NF_CT_HELPER_BUILD_BUG_ON(sizeof(struct nf_ct_ftp_master));
 
-       ftp_buffer = kmalloc(65536, GFP_KERNEL);
-       if (!ftp_buffer)
-               return -ENOMEM;
-
        if (ports_c == 0)
                ports[ports_c++] = FTP_PORT;
 
@@ -603,7 +592,6 @@ static int __init nf_conntrack_ftp_init(void)
        ret = nf_conntrack_helpers_register(ftp, ports_c * 2);
        if (ret < 0) {
                pr_err("failed to register helpers\n");
-               kfree(ftp_buffer);
                return ret;
        }