[PATCH net-next] ipvs: properly dereference pe in ip_vs_add_service

[PATCH net-next] ipvs: properly dereference pe in ip_vs_add_service

[Chen Hanxiao]

Use rcu_dereference_protected to resolve sparse warning:

  net/netfilter/ipvs/ip_vs_ctl.c:1471:27: warning: dereference of noderef expression

Fixes: 39b972231536 ("ipvs: handle connections started by real-servers")
Signed-off-by: Chen Hanxiao <chenhx.fnst@fujitsu.com>
---
 net/netfilter/ipvs/ip_vs_ctl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index b6d0dcf3a5c3..925e2143ba15 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1369,7 +1369,7 @@ ip_vs_add_service(struct netns_ipvs *ipvs, struct ip_vs_service_user_kern *u,
 {
 	int ret = 0;
 	struct ip_vs_scheduler *sched = NULL;
-	struct ip_vs_pe *pe = NULL;
+	struct ip_vs_pe *pe = NULL, *tmp_pe = NULL;
 	struct ip_vs_service *svc = NULL;
 	int ret_hooks = -1;
 
@@ -1468,7 +1468,8 @@ ip_vs_add_service(struct netns_ipvs *ipvs, struct ip_vs_service_user_kern *u,
 		atomic_inc(&ipvs->ftpsvc_counter);
 	else if (svc->port == 0)
 		atomic_inc(&ipvs->nullsvc_counter);
-	if (svc->pe && svc->pe->conn_out)
+	tmp_pe = rcu_dereference_protected(svc->pe, 1);
+	if (tmp_pe && tmp_pe->conn_out)
 		atomic_inc(&ipvs->conn_out_counter);
 
 	/* Count only IPv4 services for old get/setsockopt interface */
-- 
2.39.1

Re: [PATCH net-next] ipvs: properly dereference pe in ip_vs_add_service

[Julian Anastasov]

	Hello,

On Wed, 26 Jun 2024, Chen Hanxiao wrote:

> Use rcu_dereference_protected to resolve sparse warning:
> 
>   net/netfilter/ipvs/ip_vs_ctl.c:1471:27: warning: dereference of noderef expression
> 
> Fixes: 39b972231536 ("ipvs: handle connections started by real-servers")
> Signed-off-by: Chen Hanxiao <chenhx.fnst@fujitsu.com>
> ---
>  net/netfilter/ipvs/ip_vs_ctl.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index b6d0dcf3a5c3..925e2143ba15 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -1369,7 +1369,7 @@ ip_vs_add_service(struct netns_ipvs *ipvs, struct ip_vs_service_user_kern *u,
>  {
>  	int ret = 0;
>  	struct ip_vs_scheduler *sched = NULL;
> -	struct ip_vs_pe *pe = NULL;
> +	struct ip_vs_pe *pe = NULL, *tmp_pe = NULL;

	NULL init is not needed

>  	struct ip_vs_service *svc = NULL;
>  	int ret_hooks = -1;
>  
> @@ -1468,7 +1468,8 @@ ip_vs_add_service(struct netns_ipvs *ipvs, struct ip_vs_service_user_kern *u,
>  		atomic_inc(&ipvs->ftpsvc_counter);
>  	else if (svc->port == 0)
>  		atomic_inc(&ipvs->nullsvc_counter);
> -	if (svc->pe && svc->pe->conn_out)
> +	tmp_pe = rcu_dereference_protected(svc->pe, 1);
> +	if (tmp_pe && tmp_pe->conn_out)
>  		atomic_inc(&ipvs->conn_out_counter);

	Alternative option would be to use 'pe' above and to move
the RCU_INIT_POINTER and pe = NULL with their comment here.
It is up to you to decide which option is better...

Regards

--
Julian Anastasov <ja@ssi.bg>

回复: [PATCH net-next] ipvs: properly dereference pe in ip_vs_add_service

[Hanxiao Chen (Fujitsu)]

> -----邮件原件-----
> 发件人: Julian Anastasov <ja@ssi.bg>
> 发送时间: 2024年6月27日 1:54
> 收件人: Chen, Hanxiao<chenhx.fnst@fujitsu.com>
> 抄送: Simon Horman <horms@verge.net.au>; Pablo Neira Ayuso
> <pablo@netfilter.org>; Jozsef Kadlecsik <kadlec@netfilter.org>; David S . Miller
> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>;
> netdev@vger.kernel.org; lvs-devel@vger.kernel.org;
> netfilter-devel@vger.kernel.org
> 主题: Re: [PATCH net-next] ipvs: properly dereference pe in ip_vs_add_service
> 
> 
> 	Hello,
> 
> On Wed, 26 Jun 2024, Chen Hanxiao wrote:
> 
> > Use rcu_dereference_protected to resolve sparse warning:
> >
> >   net/netfilter/ipvs/ip_vs_ctl.c:1471:27: warning: dereference of noderef
> expression
> >
> > Fixes: 39b972231536 ("ipvs: handle connections started by real-servers")
> > Signed-off-by: Chen Hanxiao <chenhx.fnst@fujitsu.com>
> > ---
...
> > -	if (svc->pe && svc->pe->conn_out)
> > +	tmp_pe = rcu_dereference_protected(svc->pe, 1);
> > +	if (tmp_pe && tmp_pe->conn_out)
> >  		atomic_inc(&ipvs->conn_out_counter);
> 
> 	Alternative option would be to use 'pe' above and to move
> the RCU_INIT_POINTER and pe = NULL with their comment here.
> It is up to you to decide which option is better...
> 
Thanks for the advice.
Using pe instead of RCU dereference looks like a better choice.
v2 will come soon.

Regards,
- Chen

Re: [PATCH net-next] ipvs: properly dereference pe in ip_vs_add_service

[Ratheesh Kannoth]

On 2024-06-26 at 13:41:59, Chen Hanxiao (chenhx.fnst@fujitsu.com) wrote:
>
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index b6d0dcf3a5c3..925e2143ba15 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -1369,7 +1369,7 @@ ip_vs_add_service(struct netns_ipvs *ipvs, struct ip_vs_service_user_kern *u,
>  {
>  	int ret = 0;
>  	struct ip_vs_scheduler *sched = NULL;
> -	struct ip_vs_pe *pe = NULL;
> +	struct ip_vs_pe *pe = NULL, *tmp_pe = NULL;
Reverse xmas tree.
>  	struct ip_vs_service *svc = NULL;
>  	int ret_hooks = -1;
>
>