netfilter: nf_conntrack: use rcu accessors where needed

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>
This commit is contained in:
Florian Westphal 2022-06-22 11:00:46 +02:00 committed by Pablo Neira Ayuso
parent 6976890e89
commit e14575fa75
7 changed files with 57 additions and 16 deletions

View File

@ -20,6 +20,7 @@ int nf_conntrack_broadcast_help(struct sk_buff *skb,
enum ip_conntrack_info ctinfo, enum ip_conntrack_info ctinfo,
unsigned int timeout) unsigned int timeout)
{ {
const struct nf_conntrack_helper *helper;
struct nf_conntrack_expect *exp; struct nf_conntrack_expect *exp;
struct iphdr *iph = ip_hdr(skb); struct iphdr *iph = ip_hdr(skb);
struct rtable *rt = skb_rtable(skb); struct rtable *rt = skb_rtable(skb);
@ -58,7 +59,10 @@ int nf_conntrack_broadcast_help(struct sk_buff *skb,
goto out; goto out;
exp->tuple = ct->tuplehash[IP_CT_DIR_REPLY].tuple; 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.u3.ip = mask;
exp->mask.src.u.udp.port = htons(0xFFFF); exp->mask.src.u.udp.port = htons(0xFFFF);

View File

@ -249,7 +249,7 @@ int __nf_ct_try_assign_helper(struct nf_conn *ct, struct nf_conn *tmpl,
if (tmpl != NULL) { if (tmpl != NULL) {
help = nfct_help(tmpl); help = nfct_help(tmpl);
if (help != NULL) { if (help != NULL) {
helper = help->helper; helper = rcu_dereference(help->helper);
set_bit(IPS_HELPER_BIT, &ct->status); set_bit(IPS_HELPER_BIT, &ct->status);
} }
} }

View File

@ -2004,7 +2004,7 @@ static int ctnetlink_change_helper(struct nf_conn *ct,
} }
if (help) { if (help) {
if (help->helper == helper) { if (rcu_access_pointer(help->helper) == helper) {
/* update private helper data if allowed. */ /* update private helper data if allowed. */
if (helper->from_nlattr) if (helper->from_nlattr)
helper->from_nlattr(helpinfo, ct); 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) 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 struct nf_conn_help *m_help;
const char *name = data; const char *name = data;
m_help = nfct_help(exp->master); 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) static bool expect_iter_all(struct nf_conntrack_expect *exp, void *data)

View File

@ -1229,6 +1229,7 @@ static int process_register_request(struct sk_buff *skb, unsigned int protoff,
struct nf_conntrack_expect *exp; struct nf_conntrack_expect *exp;
union nf_inet_addr *saddr, daddr; union nf_inet_addr *saddr, daddr;
const struct nf_nat_sip_hooks *hooks; const struct nf_nat_sip_hooks *hooks;
struct nf_conntrack_helper *helper;
__be16 port; __be16 port;
u8 proto; u8 proto;
unsigned int expires = 0; unsigned int expires = 0;
@ -1289,10 +1290,14 @@ static int process_register_request(struct sk_buff *skb, unsigned int protoff,
if (sip_direct_signalling) if (sip_direct_signalling)
saddr = &ct->tuplehash[!dir].tuple.src.u3; 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), nf_ct_expect_init(exp, SIP_EXPECT_SIGNALLING, nf_ct_l3num(ct),
saddr, &daddr, proto, NULL, &port); saddr, &daddr, proto, NULL, &port);
exp->timeout.expires = sip_timeout * HZ; 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; exp->flags = NF_CT_EXPECT_PERMANENT | NF_CT_EXPECT_INACTIVE;
hooks = rcu_dereference(nf_nat_sip_hooks); hooks = rcu_dereference(nf_nat_sip_hooks);

View 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); struct nf_conn_timeout *timeout_ext = nf_ct_timeout_find(ct);
if (timeout_ext && (!timeout || timeout_ext->timeout == timeout)) 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); RCU_INIT_POINTER(timeout_ext->timeout, NULL);
}
/* We are not intended to delete this conntrack. */ /* We are not intended to delete this conntrack. */
return 0; return 0;
@ -127,7 +133,11 @@ void nf_ct_destroy_timeout(struct nf_conn *ct)
if (h) { if (h) {
timeout_ext = nf_ct_timeout_find(ct); timeout_ext = nf_ct_timeout_find(ct);
if (timeout_ext) { 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); RCU_INIT_POINTER(timeout_ext->timeout, NULL);
} }
} }

View File

@ -96,11 +96,13 @@ static int
nfnl_cthelper_from_nlattr(struct nlattr *attr, struct nf_conn *ct) nfnl_cthelper_from_nlattr(struct nlattr *attr, struct nf_conn *ct)
{ {
struct nf_conn_help *help = nfct_help(ct); struct nf_conn_help *help = nfct_help(ct);
const struct nf_conntrack_helper *helper;
if (attr == NULL) if (attr == NULL)
return -EINVAL; return -EINVAL;
if (help->helper->data_len == 0) helper = rcu_dereference(help->helper);
if (!helper || helper->data_len == 0)
return -EINVAL; return -EINVAL;
nla_memcpy(help->data, attr, sizeof(help->data)); 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) 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_conn_help *help = nfct_help(ct);
const struct nf_conntrack_helper *helper;
if (help->helper->data_len && helper = rcu_dereference(help->helper);
nla_put(skb, CTA_HELP_INFO, help->helper->data_len, &help->data)) if (helper && helper->data_len &&
nla_put(skb, CTA_HELP_INFO, helper->data_len, &help->data))
goto nla_put_failure; goto nla_put_failure;
return 0; return 0;

View File

@ -96,7 +96,7 @@ xt_ct_set_helper(struct nf_conn *ct, const char *helper_name,
return -ENOMEM; return -ENOMEM;
} }
help->helper = helper; rcu_assign_pointer(help->helper, helper);
return 0; 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, static int xt_ct_tg_check(const struct xt_tgchk_param *par,
struct xt_ct_target_info_v1 *info) struct xt_ct_target_info_v1 *info)
{ {
@ -207,8 +222,7 @@ out:
err4: err4:
help = nfct_help(ct); help = nfct_help(ct);
if (help) xt_ct_put_helper(help);
nf_conntrack_helper_put(help->helper);
err3: err3:
nf_ct_tmpl_free(ct); nf_ct_tmpl_free(ct);
err2: err2:
@ -270,8 +284,7 @@ static void xt_ct_tg_destroy(const struct xt_tgdtor_param *par,
if (ct) { if (ct) {
help = nfct_help(ct); help = nfct_help(ct);
if (help) xt_ct_put_helper(help);
nf_conntrack_helper_put(help->helper);
nf_ct_netns_put(par->net, par->family); nf_ct_netns_put(par->net, par->family);