Re: [PATCH] ip6_output: ensure flow saddr actually belongs to device

2016-11-13 Thread David Ahern
On 11/13/16 1:19 PM, Jason A. Donenfeld wrote:
> I gave v2 my best shot. Hopefully it's adequate, but I have a feeling
> it might be best for you to just code up what you have in mind.

nah, you are doing fine. one more comment on v2.


Re: [PATCH] ip6_output: ensure flow saddr actually belongs to device

2016-11-13 Thread Jason A. Donenfeld
Hi David,

On Sun, Nov 13, 2016 at 5:30 PM, David Ahern  wrote:
> You can't require the address to be on the dst device. e.g., it can be an 
> address from the loopback/vrf device.
>
> This block needs to be done at function entry, and pass dev as NULL to mean 
> is the address assigned to any interface. That gets you the equivalency of 
> the IPv4 check.

I gave v2 my best shot. Hopefully it's adequate, but I have a feeling
it might be best for you to just code up what you have in mind.

Regards,
Jason


Re: [PATCH] ip6_output: ensure flow saddr actually belongs to device

2016-11-13 Thread David Ahern
On 11/13/16 6:23 AM, Jason A. Donenfeld wrote:
> This puts the IPv6 routing functions in parity with the IPv4 routing
> functions. Namely, we now check in v6 that if a flowi6 requests an
> saddr, the returned dst actually corresponds to a net device that has
> that saddr. This mirrors the v4 logic with __ip_dev_find in
> __ip_route_output_key_hash. In the event that the returned dst is not
> for a dst with a dev that has the saddr, we return -EINVAL, just like
> v4; this makes it easy to use the same error handlers for both cases.
> 
> Signed-off-by: Jason A. Donenfeld 
> Cc: David Ahern 
> ---
>  net/ipv6/ip6_output.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 6001e78..a834129 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -1011,6 +1011,11 @@ static int ip6_dst_lookup_tail(struct net *net, const 
> struct sock *sk,
>   }
>   }
>  #endif
> + if (!ipv6_addr_any(&fl6->saddr) &&
> + !ipv6_chk_addr(net, &fl6->saddr, (*dst)->dev, 1)) {
> + err = -EINVAL;
> + goto out_err_release;
> + }
>  
>   return 0;

You can't require the address to be on the dst device. e.g., it can be an 
address from the loopback/vrf device.

This block needs to be done at function entry, and pass dev as NULL to mean is 
the address assigned to any interface. That gets you the equivalency of the 
IPv4 check.


[PATCH] ip6_output: ensure flow saddr actually belongs to device

2016-11-13 Thread Jason A. Donenfeld
This puts the IPv6 routing functions in parity with the IPv4 routing
functions. Namely, we now check in v6 that if a flowi6 requests an
saddr, the returned dst actually corresponds to a net device that has
that saddr. This mirrors the v4 logic with __ip_dev_find in
__ip_route_output_key_hash. In the event that the returned dst is not
for a dst with a dev that has the saddr, we return -EINVAL, just like
v4; this makes it easy to use the same error handlers for both cases.

Signed-off-by: Jason A. Donenfeld 
Cc: David Ahern 
---
 net/ipv6/ip6_output.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 6001e78..a834129 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1011,6 +1011,11 @@ static int ip6_dst_lookup_tail(struct net *net, const 
struct sock *sk,
}
}
 #endif
+   if (!ipv6_addr_any(&fl6->saddr) &&
+   !ipv6_chk_addr(net, &fl6->saddr, (*dst)->dev, 1)) {
+   err = -EINVAL;
+   goto out_err_release;
+   }
 
return 0;
 
-- 
2.10.2