netfilter: nf_conntrack_expect: use expect->helper

Use expect->helper in ctnetlink and /proc to dump the helper name.
Using nfct_help() without holding a reference to the master conntrack
is unsafe.

Use exp->master->helper in ctnetlink path if userspace does not provide
an explicit helper when creating an expectation to retain the existing
behaviour. The ctnetlink expectation path holds the reference on the
master conntrack and nf_conntrack_expect lock and the nfnetlink glue
path refers to the master ct that is attached to the skb.

Reported-by: Hyunwoo Kim <imv4bel@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/net/netfilter/nf_conntrack_expect.c b/net/netfilter/nf_conntrack_expect.c
index 841e316240dae96bcb9b57fa06a84c95650f14ec..64977db12b1d68e7f0dc3eed5ce761f7c8d1dc94 100644 (file)
--- a/net/netfilter/nf_conntrack_expect.c
+++ b/net/netfilter/nf_conntrack_expect.c
@@ -666,7 +666,7 @@ static int exp_seq_show(struct seq_file *s, void *v)
        if (expect->flags & NF_CT_EXPECT_USERSPACE)
                seq_printf(s, "%sUSERSPACE", delim);
 
-       helper = rcu_dereference(nfct_help(expect->master)->helper);
+       helper = rcu_dereference(expect->helper);
        if (helper) {
                seq_printf(s, "%s%s", expect->flags ? " " : "", helper->name);
                if (helper->expect_policy[expect->class].name[0])

diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c
index 294a6ffcbccdd95abdcb203f73a813d8098c7fa0..1b330ba6613bb2c6d37dabdced8df03c179292ba 100644 (file)
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -395,14 +395,10 @@ EXPORT_SYMBOL_GPL(nf_conntrack_helper_register);
 
 static bool expect_iter_me(struct nf_conntrack_expect *exp, void *data)
 {
-       struct nf_conn_help *help = nfct_help(exp->master);
        const struct nf_conntrack_helper *me = data;
        const struct nf_conntrack_helper *this;
 
-       if (rcu_access_pointer(exp->helper) == me)
-               return true;
-
-       this = rcu_dereference_protected(help->helper,
+       this = rcu_dereference_protected(exp->helper,
                                         lockdep_is_held(&nf_conntrack_expect_lock));
        return this == me;
 }

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index a42d142907862cab8c9773a38a0d49dee5bde620..8477c3736432885df2a2c76e439749a3c75e6695 100644 (file)
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -3012,7 +3012,7 @@ ctnetlink_exp_dump_expect(struct sk_buff *skb,
 {
        struct nf_conn *master = exp->master;
        long timeout = ((long)exp->timeout.expires - (long)jiffies) / HZ;
-       struct nf_conn_help *help;
+       struct nf_conntrack_helper *helper;
 #if IS_ENABLED(CONFIG_NF_NAT)
        struct nlattr *nest_parms;
        struct nf_conntrack_tuple nat_tuple = {};
@@ -3057,15 +3057,12 @@ ctnetlink_exp_dump_expect(struct sk_buff *skb,
            nla_put_be32(skb, CTA_EXPECT_FLAGS, htonl(exp->flags)) ||
            nla_put_be32(skb, CTA_EXPECT_CLASS, htonl(exp->class)))
                goto nla_put_failure;
-       help = nfct_help(master);
-       if (help) {
-               struct nf_conntrack_helper *helper;
 
-               helper = rcu_dereference(help->helper);
-               if (helper &&
-                   nla_put_string(skb, CTA_EXPECT_HELP_NAME, helper->name))
-                       goto nla_put_failure;
-       }
+       helper = rcu_dereference(exp->helper);
+       if (helper &&
+           nla_put_string(skb, CTA_EXPECT_HELP_NAME, helper->name))
+               goto nla_put_failure;
+
        expfn = nf_ct_helper_expectfn_find_by_symbol(exp->expectfn);
        if (expfn != NULL &&
            nla_put_string(skb, CTA_EXPECT_FN, expfn->name))
@@ -3394,12 +3391,9 @@ static int ctnetlink_get_expect(struct sk_buff *skb,
 static bool expect_iter_name(struct nf_conntrack_expect *exp, void *data)
 {
        struct nf_conntrack_helper *helper;
-       const struct nf_conn_help *m_help;
        const char *name = data;
 
-       m_help = nfct_help(exp->master);
-
-       helper = rcu_dereference(m_help->helper);
+       helper = rcu_dereference(exp->helper);
        if (!helper)
                return false;
 
@@ -3534,9 +3528,9 @@ ctnetlink_alloc_expect(const struct nlattr * const cda[], struct nf_conn *ct,
                       struct nf_conntrack_tuple *tuple,
                       struct nf_conntrack_tuple *mask)
 {
-       u_int32_t class = 0;
        struct nf_conntrack_expect *exp;
        struct nf_conn_help *help;
+       u32 class = 0;
        int err;
 
        help = nfct_help(ct);
@@ -3573,6 +3567,8 @@ ctnetlink_alloc_expect(const struct nlattr * const cda[], struct nf_conn *ct,
 
        exp->class = class;
        exp->master = ct;
+       if (!helper)
+               helper = rcu_dereference(help->helper);
        rcu_assign_pointer(exp->helper, helper);
        exp->tuple = *tuple;
        exp->mask.src.u3 = mask->src.u3;

diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
index 106b2f419e1956593b7b7faf74ea8bfe78da6468..20e57cf5c83a9550a89abb32c398b2b96f74e95c 100644 (file)
--- a/net/netfilter/nf_conntrack_sip.c
+++ b/net/netfilter/nf_conntrack_sip.c
@@ -924,7 +924,7 @@ static int set_expected_rtp_rtcp(struct sk_buff *skb, unsigned int protoff,
                exp = __nf_ct_expect_find(net, nf_ct_zone(ct), &tuple);
 
                if (!exp || exp->master == ct ||
-                   nfct_help(exp->master)->helper != nfct_help(ct)->helper ||
+                   exp->helper != nfct_help(ct)->helper ||
                    exp->class != class)
                        break;
 #if IS_ENABLED(CONFIG_NF_NAT)