[PATCH][nf-next] netfilter: Use RCU primitives under RCU protected data

[PATCH][nf-next] netfilter: Use RCU primitives under RCU protected data

[Li RongQing]

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));
 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);
 
-- 
2.16.2

Re: [PATCH][nf-next] netfilter: Use RCU primitives under RCU protected data

[Eric Dumazet]

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);
>  
> 

答复: [PATCH][nf-next] netfilter: Use RCU primitives under RCU protected data

[Li,Rongqing]

> -----邮件原件-----
> 发件人: 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