Message ID | 1551065562-26220-1-git-send-email-lirongqing@baidu.com |
---|---|
State | Changes Requested |
Delegated to: | Pablo Neira |
Headers | show |
Series | [nf-next] netfilter: Use RCU primitives under RCU protected data | expand |
On 02/24/2019 07:32 PM, Li RongQing wrote: > This pointer is RCU protected, so proper primitives should be used. > > Signed-off-by: Zhang Yu <zhangyu31@baidu.com> > Signed-off-by: Li RongQing <lirongqing@baidu.com> > --- > net/netfilter/xt_CT.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c > index 0fa863f57575..26dee8bba326 100644 > --- a/net/netfilter/xt_CT.c > +++ b/net/netfilter/xt_CT.c > @@ -99,7 +99,7 @@ xt_ct_set_helper(struct nf_conn *ct, const char *helper_name, > return -ENOMEM; > } > > - help->helper = helper; > + RCU_INIT_POINTER(help->helper, helper); > return 0; > } > > @@ -274,7 +274,7 @@ static int xt_ct_tg_check(const struct xt_tgchk_param *par, > err4: > help = nfct_help(ct); > if (help) > - nf_conntrack_helper_put(help->helper); > + nf_conntrack_helper_put(rcu_dereference(help->helper)); Are you sure it is ok, have you tested your patch ? Where is the rcu_read_lock() ? CONFIG_PROVE_RCU=y will likely give a splat here. > err3: > nf_ct_tmpl_free(ct); > err2: > @@ -357,7 +357,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); > + nf_conntrack_helper_put(rcu_dereference(help->helper)); Same here. > > nf_ct_netns_put(par->net, par->family); > >
> -----邮件原件----- > 发件人: Eric Dumazet [mailto:eric.dumazet@gmail.com] > 发送时间: 2019年2月25日 11:53 > 收件人: Li,Rongqing <lirongqing@baidu.com>; netfilter-devel@vger.kernel.org > 主题: Re: [PATCH][nf-next] netfilter: Use RCU primitives under RCU protected > data > > > > On 02/24/2019 07:32 PM, Li RongQing wrote: > > This pointer is RCU protected, so proper primitives should be used. > > > > Signed-off-by: Zhang Yu <zhangyu31@baidu.com> > > Signed-off-by: Li RongQing <lirongqing@baidu.com> > > --- > > net/netfilter/xt_CT.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c index > > 0fa863f57575..26dee8bba326 100644 > > --- a/net/netfilter/xt_CT.c > > +++ b/net/netfilter/xt_CT.c > > @@ -99,7 +99,7 @@ xt_ct_set_helper(struct nf_conn *ct, const char > *helper_name, > > return -ENOMEM; > > } > > > > - help->helper = helper; > > + RCU_INIT_POINTER(help->helper, helper); > > return 0; > > } > > > > @@ -274,7 +274,7 @@ static int xt_ct_tg_check(const struct > > xt_tgchk_param *par, > > err4: > > help = nfct_help(ct); > > if (help) > > - nf_conntrack_helper_put(help->helper); > > + nf_conntrack_helper_put(rcu_dereference(help->helper)); > > Are you sure it is ok, have you tested your patch ? > > Where is the rcu_read_lock() ? > > CONFIG_PROVE_RCU=y will likely give a splat here. > Thanks, I will test it as your suggestion -RongQing
diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c index 0fa863f57575..26dee8bba326 100644 --- a/net/netfilter/xt_CT.c +++ b/net/netfilter/xt_CT.c @@ -99,7 +99,7 @@ xt_ct_set_helper(struct nf_conn *ct, const char *helper_name, return -ENOMEM; } - help->helper = helper; + RCU_INIT_POINTER(help->helper, helper); return 0; } @@ -274,7 +274,7 @@ static int xt_ct_tg_check(const struct xt_tgchk_param *par, err4: help = nfct_help(ct); if (help) - nf_conntrack_helper_put(help->helper); + nf_conntrack_helper_put(rcu_dereference(help->helper)); err3: nf_ct_tmpl_free(ct); err2: @@ -357,7 +357,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); + nf_conntrack_helper_put(rcu_dereference(help->helper)); nf_ct_netns_put(par->net, par->family);