LVS
lvs-devel
Google
 
Web LinuxVirtualServer.org

Re: [Linux-kernel-mentees] [PATCH net-next v2] ipvs: Fix uninit-value in

To: Peilin Ye <yepeilin.cs@xxxxxxxxx>
Subject: Re: [Linux-kernel-mentees] [PATCH net-next v2] ipvs: Fix uninit-value in do_ip_vs_set_ctl()
Cc: Wensong Zhang <wensong@xxxxxxxxxxxx>, Simon Horman <horms@xxxxxxxxxxxx>, Cong Wang <xiyou.wangcong@xxxxxxxxx>, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>, Jozsef Kadlecsik <kadlec@xxxxxxxxxxxxx>, Florian Westphal <fw@xxxxxxxxx>, "David S. Miller" <davem@xxxxxxxxxxxxx>, Jakub Kicinski <kuba@xxxxxxxxxx>, Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>, netdev@xxxxxxxxxxxxxxx, lvs-devel@xxxxxxxxxxxxxxx, netfilter-devel@xxxxxxxxxxxxxxx, coreteam@xxxxxxxxxxxxx, linux-kernel-mentees@xxxxxxxxxxxxxxxxxxxxxxxxx, syzkaller-bugs@xxxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx
From: Julian Anastasov <ja@xxxxxx>
Date: Tue, 11 Aug 2020 13:29:04 +0300 (EEST)
        Hello,

On Tue, 11 Aug 2020, Peilin Ye wrote:

> do_ip_vs_set_ctl() is referencing uninitialized stack value when `len` is
> zero. Fix it.
> 
> Reported-by: syzbot+23b5f9e7caf61d9a3898@xxxxxxxxxxxxxxxxxxxxxxxxx
> Link: 
> https://syzkaller.appspot.com/bug?id=46ebfb92a8a812621a001ef04d90dfa459520fe2
> Suggested-by: Julian Anastasov <ja@xxxxxx>
> Signed-off-by: Peilin Ye <yepeilin.cs@xxxxxxxxx>

        Looks good to me, thanks!

Acked-by: Julian Anastasov <ja@xxxxxx>

> ---
> Changes in v2:
>     - Target net-next tree. (Suggested by Julian Anastasov <ja@xxxxxx>)
>     - Reject all `len == 0` requests except `IP_VS_SO_SET_FLUSH`, instead
>       of initializing `arg`. (Suggested by Cong Wang
>       <xiyou.wangcong@xxxxxxxxx>, Julian Anastasov <ja@xxxxxx>)
> 
>  net/netfilter/ipvs/ip_vs_ctl.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 412656c34f20..beeafa42aad7 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -2471,6 +2471,10 @@ do_ip_vs_set_ctl(struct sock *sk, int cmd, void __user 
> *user, unsigned int len)
>               /* Set timeout values for (tcp tcpfin udp) */
>               ret = ip_vs_set_timeout(ipvs, (struct ip_vs_timeout_user *)arg);
>               goto out_unlock;
> +     } else if (!len) {
> +             /* No more commands with len == 0 below */
> +             ret = -EINVAL;
> +             goto out_unlock;
>       }
>  
>       usvc_compat = (struct ip_vs_service_user *)arg;
> @@ -2547,9 +2551,6 @@ do_ip_vs_set_ctl(struct sock *sk, int cmd, void __user 
> *user, unsigned int len)
>               break;
>       case IP_VS_SO_SET_DELDEST:
>               ret = ip_vs_del_dest(svc, &udest);
> -             break;
> -     default:
> -             ret = -EINVAL;
>       }
>  
>    out_unlock:
> -- 
> 2.25.1

Regards

--
Julian Anastasov <ja@xxxxxx>

<Prev in Thread] Current Thread [Next in Thread>