netfilter: nf_conntrack: use rcu accessors where needed
authorFlorian Westphal <fw@strlen.de>
Wed, 22 Jun 2022 09:00:46 +0000 (11:00 +0200)
committerPablo Neira Ayuso <pablo@netfilter.org>
Mon, 11 Jul 2022 14:25:15 +0000 (16:25 +0200)
Sparse complains about direct access to the 'helper' and timeout members.
Both have __rcu annotation, so use the accessors.

xt_CT is fine, accesses occur before the structure is visible to other
cpus.  Switch to rcu accessors there as well to reduce noise.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
net/netfilter/nf_conntrack_broadcast.c
net/netfilter/nf_conntrack_helper.c
net/netfilter/nf_conntrack_netlink.c
net/netfilter/nf_conntrack_sip.c
net/netfilter/nf_conntrack_timeout.c
net/netfilter/nfnetlink_cthelper.c
net/netfilter/xt_CT.c

index 1ba6becc30795a0080a39b9149849af1905f02ae..9fb9b80312989beec91d80bdea6ddc344c2b3fa8 100644 (file)
@@ -20,6 +20,7 @@ int nf_conntrack_broadcast_help(struct sk_buff *skb,
                                enum ip_conntrack_info ctinfo,
                                unsigned int timeout)
 {
+       const struct nf_conntrack_helper *helper;
        struct nf_conntrack_expect *exp;
        struct iphdr *iph = ip_hdr(skb);
        struct rtable *rt = skb_rtable(skb);
@@ -58,7 +59,10 @@ int nf_conntrack_broadcast_help(struct sk_buff *skb,
                goto out;
 
        exp->tuple                = ct->tuplehash[IP_CT_DIR_REPLY].tuple;
-       exp->tuple.src.u.udp.port = help->helper->tuple.src.u.udp.port;
+
+       helper = rcu_dereference(help->helper);
+       if (helper)
+               exp->tuple.src.u.udp.port = helper->tuple.src.u.udp.port;
 
        exp->mask.src.u3.ip       = mask;
        exp->mask.src.u.udp.port  = htons(0xFFFF);
index 1e0424d37abc716b8921997852fe65b39fccdc75..e96b3222144471abe75925340fa5824c66545a5a 100644 (file)
@@ -249,7 +249,7 @@ int __nf_ct_try_assign_helper(struct nf_conn *ct, struct nf_conn *tmpl,
        if (tmpl != NULL) {
                help = nfct_help(tmpl);
                if (help != NULL) {
-                       helper = help->helper;
+                       helper = rcu_dereference(help->helper);
                        set_bit(IPS_HELPER_BIT, &ct->status);
                }
        }
index 722af5e309ba280a17553e0d4164dcd25d0754d9..25c2c0de78f0eb02bbc259e32d9ce853e9c34482 100644 (file)
@@ -2004,7 +2004,7 @@ static int ctnetlink_change_helper(struct nf_conn *ct,
        }
 
        if (help) {
-               if (help->helper == helper) {
+               if (rcu_access_pointer(help->helper) == helper) {
                        /* update private helper data if allowed. */
                        if (helper->from_nlattr)
                                helper->from_nlattr(helpinfo, ct);
@@ -3412,12 +3412,17 @@ 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);
 
-       return strcmp(m_help->helper->name, name) == 0;
+       helper = rcu_dereference(m_help->helper);
+       if (!helper)
+               return false;
+
+       return strcmp(helper->name, name) == 0;
 }
 
 static bool expect_iter_all(struct nf_conntrack_expect *exp, void *data)
index a88b43624b2708086af8f234fdead190a4b9097c..daf06f71d31cad67e020847dc745e9aa5a422c70 100644 (file)
@@ -1229,6 +1229,7 @@ static int process_register_request(struct sk_buff *skb, unsigned int protoff,
        struct nf_conntrack_expect *exp;
        union nf_inet_addr *saddr, daddr;
        const struct nf_nat_sip_hooks *hooks;
+       struct nf_conntrack_helper *helper;
        __be16 port;
        u8 proto;
        unsigned int expires = 0;
@@ -1289,10 +1290,14 @@ static int process_register_request(struct sk_buff *skb, unsigned int protoff,
        if (sip_direct_signalling)
                saddr = &ct->tuplehash[!dir].tuple.src.u3;
 
+       helper = rcu_dereference(nfct_help(ct)->helper);
+       if (!helper)
+               return NF_DROP;
+
        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 = nfct_help(ct)->helper;
+       exp->helper = helper;
        exp->flags = NF_CT_EXPECT_PERMANENT | NF_CT_EXPECT_INACTIVE;
 
        hooks = rcu_dereference(nf_nat_sip_hooks);
index 821365ed5b2c46effebad7035accc6977bd5c0bb..0cc584d3dbb1d0ac8b4c1e846e41bfcd620091b5 100644 (file)
@@ -29,8 +29,14 @@ static int untimeout(struct nf_conn *ct, void *timeout)
 {
        struct nf_conn_timeout *timeout_ext = nf_ct_timeout_find(ct);
 
-       if (timeout_ext && (!timeout || timeout_ext->timeout == timeout))
-               RCU_INIT_POINTER(timeout_ext->timeout, NULL);
+       if (timeout_ext) {
+               const struct nf_ct_timeout *t;
+
+               t = rcu_access_pointer(timeout_ext->timeout);
+
+               if (!timeout || t == timeout)
+                       RCU_INIT_POINTER(timeout_ext->timeout, NULL);
+       }
 
        /* We are not intended to delete this conntrack. */
        return 0;
@@ -127,7 +133,11 @@ void nf_ct_destroy_timeout(struct nf_conn *ct)
        if (h) {
                timeout_ext = nf_ct_timeout_find(ct);
                if (timeout_ext) {
-                       h->timeout_put(timeout_ext->timeout);
+                       struct nf_ct_timeout *t;
+
+                       t = rcu_dereference(timeout_ext->timeout);
+                       if (t)
+                               h->timeout_put(t);
                        RCU_INIT_POINTER(timeout_ext->timeout, NULL);
                }
        }
index 5c622f55c9d6842f672f8ae871d2d37cc0f82017..97248963a7d3b5aff53df9c2edd43f5c15690291 100644 (file)
@@ -96,11 +96,13 @@ static int
 nfnl_cthelper_from_nlattr(struct nlattr *attr, struct nf_conn *ct)
 {
        struct nf_conn_help *help = nfct_help(ct);
+       const struct nf_conntrack_helper *helper;
 
        if (attr == NULL)
                return -EINVAL;
 
-       if (help->helper->data_len == 0)
+       helper = rcu_dereference(help->helper);
+       if (!helper || helper->data_len == 0)
                return -EINVAL;
 
        nla_memcpy(help->data, attr, sizeof(help->data));
@@ -111,9 +113,11 @@ static int
 nfnl_cthelper_to_nlattr(struct sk_buff *skb, const struct nf_conn *ct)
 {
        const struct nf_conn_help *help = nfct_help(ct);
+       const struct nf_conntrack_helper *helper;
 
-       if (help->helper->data_len &&
-           nla_put(skb, CTA_HELP_INFO, help->helper->data_len, &help->data))
+       helper = rcu_dereference(help->helper);
+       if (helper && helper->data_len &&
+           nla_put(skb, CTA_HELP_INFO, helper->data_len, &help->data))
                goto nla_put_failure;
 
        return 0;
index 267757b0392a6451e2202c724103ae755c4673cc..2be2f7a7b60f4ec1d5bd5dfa4fc3a656825321ef 100644 (file)
@@ -96,7 +96,7 @@ xt_ct_set_helper(struct nf_conn *ct, const char *helper_name,
                return -ENOMEM;
        }
 
-       help->helper = helper;
+       rcu_assign_pointer(help->helper, helper);
        return 0;
 }
 
@@ -136,6 +136,21 @@ static u16 xt_ct_flags_to_dir(const struct xt_ct_target_info_v1 *info)
        }
 }
 
+static void xt_ct_put_helper(struct nf_conn_help *help)
+{
+       struct nf_conntrack_helper *helper;
+
+       if (!help)
+               return;
+
+       /* not yet exposed to other cpus, or ruleset
+        * already detached (post-replacement).
+        */
+       helper = rcu_dereference_raw(help->helper);
+       if (helper)
+               nf_conntrack_helper_put(helper);
+}
+
 static int xt_ct_tg_check(const struct xt_tgchk_param *par,
                          struct xt_ct_target_info_v1 *info)
 {
@@ -207,8 +222,7 @@ out:
 
 err4:
        help = nfct_help(ct);
-       if (help)
-               nf_conntrack_helper_put(help->helper);
+       xt_ct_put_helper(help);
 err3:
        nf_ct_tmpl_free(ct);
 err2:
@@ -270,8 +284,7 @@ static void xt_ct_tg_destroy(const struct xt_tgdtor_param *par,
 
        if (ct) {
                help = nfct_help(ct);
-               if (help)
-                       nf_conntrack_helper_put(help->helper);
+               xt_ct_put_helper(help);
 
                nf_ct_netns_put(par->net, par->family);