netfilter: nf_conntrack_expect: honor expectation helper field

The expectation helper field is mostly unused. As a result, the
netfilter codebase relies on accessing the helper through exp->master.

Always set on the expectation helper field so it can be used to reach
the helper.

nf_ct_expect_init() is called from packet path where the skb owns
the ct object, therefore accessing exp->master for the newly created
expectation is safe. This saves a lot of updates in all callsites
to pass the ct object as parameter to nf_ct_expect_init().

This is a preparation patches for follow up fixes.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/include/net/netfilter/nf_conntrack_expect.h b/include/net/netfilter/nf_conntrack_expect.h
index 165e7a03b8e9dccf67f315d80c81a362cbb7e5bf..1b01400b10bdb298351505e585d2129adf48d3c8 100644 (file)
--- a/include/net/netfilter/nf_conntrack_expect.h
+++ b/include/net/netfilter/nf_conntrack_expect.h
@@ -40,7 +40,7 @@ struct nf_conntrack_expect {
                         struct nf_conntrack_expect *this);
 
        /* Helper to assign to new connection */
-       struct nf_conntrack_helper *helper;
+       struct nf_conntrack_helper __rcu *helper;
 
        /* The conntrack of the master connection */
        struct nf_conn *master;

diff --git a/net/netfilter/nf_conntrack_broadcast.c b/net/netfilter/nf_conntrack_broadcast.c
index a7552a46d6acf56239131eea3828ff3250e206b3..1964c596c6468f544764bf31792e7901db2269d2 100644 (file)
--- a/net/netfilter/nf_conntrack_broadcast.c
+++ b/net/netfilter/nf_conntrack_broadcast.c
@@ -70,7 +70,7 @@ int nf_conntrack_broadcast_help(struct sk_buff *skb,
        exp->expectfn             = NULL;
        exp->flags                = NF_CT_EXPECT_PERMANENT;
        exp->class                = NF_CT_EXPECT_CLASS_DEFAULT;
-       exp->helper               = NULL;
+       rcu_assign_pointer(exp->helper, helper);
 
        nf_ct_expect_related(exp, 0);
        nf_ct_expect_put(exp);

diff --git a/net/netfilter/nf_conntrack_expect.c b/net/netfilter/nf_conntrack_expect.c
index cfc2daa3fc7f340937898b4bef0769fd31f801b5..841e316240dae96bcb9b57fa06a84c95650f14ec 100644 (file)
--- a/net/netfilter/nf_conntrack_expect.c
+++ b/net/netfilter/nf_conntrack_expect.c
@@ -309,12 +309,19 @@ struct nf_conntrack_expect *nf_ct_expect_alloc(struct nf_conn *me)
 }
 EXPORT_SYMBOL_GPL(nf_ct_expect_alloc);
 
+/* This function can only be used from packet path, where accessing
+ * master's helper is safe, because the packet holds a reference on
+ * the conntrack object. Never use it from control plane.
+ */
 void nf_ct_expect_init(struct nf_conntrack_expect *exp, unsigned int class,
                       u_int8_t family,
                       const union nf_inet_addr *saddr,
                       const union nf_inet_addr *daddr,
                       u_int8_t proto, const __be16 *src, const __be16 *dst)
 {
+       struct nf_conntrack_helper *helper = NULL;
+       struct nf_conn *ct = exp->master;
+       struct nf_conn_help *help;
        int len;
 
        if (family == AF_INET)
@@ -325,7 +332,12 @@ void nf_ct_expect_init(struct nf_conntrack_expect *exp, unsigned int class,
        exp->flags = 0;
        exp->class = class;
        exp->expectfn = NULL;
-       exp->helper = NULL;
+
+       help = nfct_help(ct);
+       if (help)
+               helper = rcu_dereference(help->helper);
+
+       rcu_assign_pointer(exp->helper, helper);
        exp->tuple.src.l3num = family;
        exp->tuple.dst.protonum = proto;
 

diff --git a/net/netfilter/nf_conntrack_h323_main.c b/net/netfilter/nf_conntrack_h323_main.c
index a2a0e22ccee198ef2421b1f8ee03cbdd2f562612..3f5c50455b716ac49299ff9f56994a222d5cabcb 100644 (file)
--- a/net/netfilter/nf_conntrack_h323_main.c
+++ b/net/netfilter/nf_conntrack_h323_main.c
@@ -643,7 +643,7 @@ static int expect_h245(struct sk_buff *skb, struct nf_conn *ct,
                          &ct->tuplehash[!dir].tuple.src.u3,
                          &ct->tuplehash[!dir].tuple.dst.u3,
                          IPPROTO_TCP, NULL, &port);
-       exp->helper = &nf_conntrack_helper_h245;
+       rcu_assign_pointer(exp->helper, &nf_conntrack_helper_h245);
 
        nathook = rcu_dereference(nfct_h323_nat_hook);
        if (memcmp(&ct->tuplehash[dir].tuple.src.u3,
@@ -767,7 +767,7 @@ static int expect_callforwarding(struct sk_buff *skb,
        nf_ct_expect_init(exp, NF_CT_EXPECT_CLASS_DEFAULT, nf_ct_l3num(ct),
                          &ct->tuplehash[!dir].tuple.src.u3, &addr,
                          IPPROTO_TCP, NULL, &port);
-       exp->helper = nf_conntrack_helper_q931;
+       rcu_assign_pointer(exp->helper, nf_conntrack_helper_q931);
 
        nathook = rcu_dereference(nfct_h323_nat_hook);
        if (memcmp(&ct->tuplehash[dir].tuple.src.u3,
@@ -1234,7 +1234,7 @@ static int expect_q931(struct sk_buff *skb, struct nf_conn *ct,
                                &ct->tuplehash[!dir].tuple.src.u3 : NULL,
                          &ct->tuplehash[!dir].tuple.dst.u3,
                          IPPROTO_TCP, NULL, &port);
-       exp->helper = nf_conntrack_helper_q931;
+       rcu_assign_pointer(exp->helper, nf_conntrack_helper_q931);
        exp->flags = NF_CT_EXPECT_PERMANENT;    /* Accept multiple calls */
 
        nathook = rcu_dereference(nfct_h323_nat_hook);
@@ -1306,7 +1306,7 @@ static int process_gcf(struct sk_buff *skb, struct nf_conn *ct,
        nf_ct_expect_init(exp, NF_CT_EXPECT_CLASS_DEFAULT, nf_ct_l3num(ct),
                          &ct->tuplehash[!dir].tuple.src.u3, &addr,
                          IPPROTO_UDP, NULL, &port);
-       exp->helper = nf_conntrack_helper_ras;
+       rcu_assign_pointer(exp->helper, nf_conntrack_helper_ras);
 
        if (nf_ct_expect_related(exp, 0) == 0) {
                pr_debug("nf_ct_ras: expect RAS ");
@@ -1523,7 +1523,7 @@ static int process_acf(struct sk_buff *skb, struct nf_conn *ct,
                          &ct->tuplehash[!dir].tuple.src.u3, &addr,
                          IPPROTO_TCP, NULL, &port);
        exp->flags = NF_CT_EXPECT_PERMANENT;
-       exp->helper = nf_conntrack_helper_q931;
+       rcu_assign_pointer(exp->helper, nf_conntrack_helper_q931);
 
        if (nf_ct_expect_related(exp, 0) == 0) {
                pr_debug("nf_ct_ras: expect Q.931 ");
@@ -1577,7 +1577,7 @@ static int process_lcf(struct sk_buff *skb, struct nf_conn *ct,
                          &ct->tuplehash[!dir].tuple.src.u3, &addr,
                          IPPROTO_TCP, NULL, &port);
        exp->flags = NF_CT_EXPECT_PERMANENT;
-       exp->helper = nf_conntrack_helper_q931;
+       rcu_assign_pointer(exp->helper, nf_conntrack_helper_q931);
 
        if (nf_ct_expect_related(exp, 0) == 0) {
                pr_debug("nf_ct_ras: expect Q.931 ");

diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c
index ceb48c3ca0a43981358f1bc61a2b47db3358e0bf..294a6ffcbccdd95abdcb203f73a813d8098c7fa0 100644 (file)
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -399,7 +399,7 @@ static bool expect_iter_me(struct nf_conntrack_expect *exp, void *data)
        const struct nf_conntrack_helper *me = data;
        const struct nf_conntrack_helper *this;
 
-       if (exp->helper == me)
+       if (rcu_access_pointer(exp->helper) == me)
                return true;
 
        this = rcu_dereference_protected(help->helper,
@@ -421,6 +421,11 @@ void nf_conntrack_helper_unregister(struct nf_conntrack_helper *me)
 
        nf_ct_expect_iterate_destroy(expect_iter_me, NULL);
        nf_ct_iterate_destroy(unhelp, me);
+
+       /* nf_ct_iterate_destroy() does an unconditional synchronize_rcu() as
+        * last step, this ensures rcu readers of exp->helper are done.
+        * No need for another synchronize_rcu() here.
+        */
 }
 EXPORT_SYMBOL_GPL(nf_conntrack_helper_unregister);
 

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index c156574e127389ab1941bd38e72cc31c1cd29363..a42d142907862cab8c9773a38a0d49dee5bde620 100644 (file)
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -3573,7 +3573,7 @@ ctnetlink_alloc_expect(const struct nlattr * const cda[], struct nf_conn *ct,
 
        exp->class = class;
        exp->master = ct;
-       exp->helper = helper;
+       rcu_assign_pointer(exp->helper, helper);
        exp->tuple = *tuple;
        exp->mask.src.u3 = mask->src.u3;
        exp->mask.src.u.all = mask->src.u.all;

diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
index 4ab5ef71d96dbbbb238a6844e5b14bfd202863e8..106b2f419e1956593b7b7faf74ea8bfe78da6468 100644 (file)
--- a/net/netfilter/nf_conntrack_sip.c
+++ b/net/netfilter/nf_conntrack_sip.c
@@ -1297,7 +1297,7 @@ static int process_register_request(struct sk_buff *skb, unsigned int protoff,
        nf_ct_expect_init(exp, SIP_EXPECT_SIGNALLING, nf_ct_l3num(ct),
                          saddr, &daddr, proto, NULL, &port);
        exp->timeout.expires = sip_timeout * HZ;
-       exp->helper = helper;
+       rcu_assign_pointer(exp->helper, helper);
        exp->flags = NF_CT_EXPECT_PERMANENT | NF_CT_EXPECT_INACTIVE;
 
        hooks = rcu_dereference(nf_nat_sip_hooks);