ipvs: add a 'default' case in do_ip_vs_set_ctl()

[PATCH v2] ipvs: add a 'default' case in do_ip_vs_set_ctl()

Posted by Li Qiong 2 years, 9 months ago

It is better to return the default switch case with
'-EINVAL', in case new commands are added. otherwise,
return a uninitialized value of ret.

Signed-off-by: Li Qiong <liqiong@nfschina.com>
Reviewed-by: Simon Horman <horms@verge.net.au>
---
v2: Add 'default' case instead of initializing 'ret'.
---
 net/netfilter/ipvs/ip_vs_ctl.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 988222fff9f0..97f6a1c8933a 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -2590,6 +2590,11 @@ do_ip_vs_set_ctl(struct sock *sk, int cmd, sockptr_t ptr, unsigned int len)
 		break;
 	case IP_VS_SO_SET_DELDEST:
 		ret = ip_vs_del_dest(svc, &udest);
+		break;
+	default:
+		WARN_ON_ONCE(1);
+		ret = -EINVAL;
+		break;
 	}
 
   out_unlock:
-- 
2.11.0

Re: [PATCH v2] ipvs: add a 'default' case in do_ip_vs_set_ctl()

Posted by Julian Anastasov 2 years, 9 months ago

	Hello,

On Mon, 12 Dec 2022, Li Qiong wrote:

> It is better to return the default switch case with
> '-EINVAL', in case new commands are added. otherwise,
> return a uninitialized value of ret.
> 
> Signed-off-by: Li Qiong <liqiong@nfschina.com>
> Reviewed-by: Simon Horman <horms@verge.net.au>

	Change looks correct to me for -next, thanks!

Acked-by: Julian Anastasov <ja@ssi.bg>

	Still, the comment can explain that this code
is currently unreachable and that some parsers need
the default case to avoid report for uninitialized 'ret'.

> ---
> v2: Add 'default' case instead of initializing 'ret'.
> ---
>  net/netfilter/ipvs/ip_vs_ctl.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 988222fff9f0..97f6a1c8933a 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -2590,6 +2590,11 @@ do_ip_vs_set_ctl(struct sock *sk, int cmd, sockptr_t ptr, unsigned int len)
>  		break;
>  	case IP_VS_SO_SET_DELDEST:
>  		ret = ip_vs_del_dest(svc, &udest);
> +		break;
> +	default:
> +		WARN_ON_ONCE(1);
> +		ret = -EINVAL;
> +		break;
>  	}
>  
>    out_unlock:
> -- 
> 2.11.0

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH v2] ipvs: add a 'default' case in do_ip_vs_set_ctl()

Posted by Pablo Neira Ayuso 2 years, 9 months ago

On Mon, Dec 12, 2022 at 04:20:41PM +0200, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Mon, 12 Dec 2022, Li Qiong wrote:
> 
> > It is better to return the default switch case with
> > '-EINVAL', in case new commands are added. otherwise,
> > return a uninitialized value of ret.
> > 
> > Signed-off-by: Li Qiong <liqiong@nfschina.com>
> > Reviewed-by: Simon Horman <horms@verge.net.au>
> 
> 	Change looks correct to me for -next, thanks!
> 
> Acked-by: Julian Anastasov <ja@ssi.bg>

Applied, thanks.