On 5/9/20 1:15 PM, Ido Schimmel wrote:
> 
> This will unfortunately cause regressions with VRFs because they don't
> work correctly with ping sockets. Simple example:

Thanks for catching this, Ido.

> 
> diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
> index 535427292194..8463b0e9e811 100644
> --- a/net/ipv4/ping.c
> +++ b/net/ipv4/ping.c
> @@ -297,6 +297,7 @@ static int ping_check_bind_addr(struct sock *sk, struct 
> inet_sock *isk,
>         struct net *net = sock_net(sk);
>         if (sk->sk_family == AF_INET) {
>                 struct sockaddr_in *addr = (struct sockaddr_in *) uaddr;
> +               u32 tb_id = RT_TABLE_LOCAL;
>                 int chk_addr_ret;
>  
>                 if (addr_len < sizeof(*addr))
> @@ -310,7 +311,15 @@ static int ping_check_bind_addr(struct sock *sk, struct 
> inet_sock *isk,
>                 pr_debug("ping_check_bind_addr(sk=%p,addr=%pI4,port=%d)\n",
>                          sk, &addr->sin_addr.s_addr, ntohs(addr->sin_port));
>  
> -               chk_addr_ret = inet_addr_type(net, addr->sin_addr.s_addr);
> +               if (sk->sk_bound_dev_if) {

that's the key point - checking sk->sk_bound_dev_if.

> +                       tb_id = l3mdev_fib_table_by_index(net,
> +                                                         
> sk->sk_bound_dev_if);
> +                       if (!tb_id)
> +                               tb_id = RT_TABLE_LOCAL;
> +               }
> +
> +               chk_addr_ret = inet_addr_type_table(net, 
> addr->sin_addr.s_addr,
> +                                                   tb_id);
>  
>                 if (addr->sin_addr.s_addr == htonl(INADDR_ANY))
>                         chk_addr_ret = RTN_LOCAL;
> 

>From a scan of the ipv4/ping.c code, ping_v4_sendmsg also does not
acknowledge sk->sk_bound_dev_if.

Reply via email to