lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Pine.LNX.4.58.0706010123020.2683@u.domain.uli>
Date:	Fri, 1 Jun 2007 02:18:28 +0300 (EEST)
From:	Julian Anastasov <ja@....bg>
To:	KOVACS Krisztian <hidden@...abit.hu>
cc:	David Miller <davem@...emloft.net>, kaber@...sh.net,
	horms@...ge.net.au, jkrzyszt@....icnet.pl, netdev@...r.kernel.org
Subject: Re: [IPV4] LVS: Allow to send ICMP unreachable responses when
 real-servers are removed


	Hello,

On Thu, 31 May 2007, KOVACS Krisztian wrote:

>   So what about this one?

	May be we can try with better coding style. Also, this version
adds undefined behavior for using FLOWI_FLAG_ANYSRC with multicast
oldflp->fl4_dst

> Loosen source address check on IPv4 output
> 
> From: KOVACS Krisztian <hidden@...abit.hu>
> 
> ip_route_output() contains a check to make sure that no flows with
> non-local source IP addresses are routed. This obviously makes using
> such addresses impossible.
> 
> This patch introduces a flowi flag which makes omitting this check
> possible. The new flag provides a way of handling transparent and
> non-transparent connections differently.
> 
> Signed-off-by: KOVACS Krisztian <hidden@...abit.hu>
> ---
> 
>  include/net/flow.h |    1 +
>  net/ipv4/route.c   |   47 +++++++++++++++++++++++++----------------------
>  2 files changed, 26 insertions(+), 22 deletions(-)
> 
> diff --git a/include/net/flow.h b/include/net/flow.h
> index f3cc1f8..1bfc0dc 100644
> --- a/include/net/flow.h
> +++ b/include/net/flow.h
> @@ -49,6 +49,7 @@ struct flowi {
>  	__u8	proto;
>  	__u8	flags;
>  #define FLOWI_FLAG_MULTIPATHOLDROUTE 0x01
> +#define FLOWI_FLAG_ANYSRC 0x02
>  	union {
>  		struct {
>  			__be16	sport;
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 8603cfb..88d0a79 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -2396,7 +2396,7 @@ static int ip_route_output_slow(struct rtable **rp, const struct flowi *oldflp)
>  
>  		/* It is equivalent to inet_addr_type(saddr) == RTN_LOCAL */
>  		dev_out = ip_dev_find(oldflp->fl4_src);
> -		if (dev_out == NULL)
> +		if (dev_out == NULL && !(oldflp->flags & FLOWI_FLAG_ANYSRC))
>  			goto out;
>  
>  		/* I removed check for oif == dev_out->oif here.
> @@ -2407,29 +2407,32 @@ static int ip_route_output_slow(struct rtable **rp, const struct flowi *oldflp)
>  		      of another iface. --ANK
>  		 */
>  
> -		if (oldflp->oif == 0
> -		    && (MULTICAST(oldflp->fl4_dst) || oldflp->fl4_dst == htonl(0xFFFFFFFF))) {
> -			/* Special hack: user can direct multicasts
> -			   and limited broadcast via necessary interface
> -			   without fiddling with IP_MULTICAST_IF or IP_PKTINFO.
> -			   This hack is not just for fun, it allows
> -			   vic,vat and friends to work.
> -			   They bind socket to loopback, set ttl to zero
> -			   and expect that it will work.
> -			   From the viewpoint of routing cache they are broken,
> -			   because we are not allowed to build multicast path
> -			   with loopback source addr (look, routing cache
> -			   cannot know, that ttl is zero, so that packet
> -			   will not leave this host and route is valid).
> -			   Luckily, this hack is good workaround.
> -			 */
> +		if (dev_out) {
> +			if (oldflp->oif == 0
> +			    && (MULTICAST(oldflp->fl4_dst)
> +				|| oldflp->fl4_dst == htonl(0xFFFFFFFF))) {
> +				/* Special hack: user can direct multicasts
> +				   and limited broadcast via necessary interface
> +				   without fiddling with IP_MULTICAST_IF or IP_PKTINFO.
> +				   This hack is not just for fun, it allows
> +				   vic,vat and friends to work.
> +				   They bind socket to loopback, set ttl to zero
> +				   and expect that it will work.
> +				   From the viewpoint of routing cache they are broken,
> +				   because we are not allowed to build multicast path
> +				   with loopback source addr (look, routing cache
> +				   cannot know, that ttl is zero, so that packet
> +				   will not leave this host and route is valid).
> +				   Luckily, this hack is good workaround.
> +				*/
> +
> +				fl.oif = dev_out->ifindex;
> +				goto make_route;
> +			}
>  
> -			fl.oif = dev_out->ifindex;
> -			goto make_route;
> -		}
> -		if (dev_out)
>  			dev_put(dev_out);
> -		dev_out = NULL;
> +			dev_out = NULL;
> +		}
>  	}

	What about something like this, it even reduces checks
in the fast path. You can post new version if the following change
looks good to you and to other developers. If additional sign line is 
needed here it is:

Signed-off-by: Julian Anastasov <ja@....bg>

@@ -2396,8 +2396,6 @@ static int ip_route_output_slow(struct r
 
 		/* It is equivalent to inet_addr_type(saddr) == RTN_LOCAL */
 		dev_out = ip_dev_find(oldflp->fl4_src);
-		if (dev_out == NULL)
-			goto out;
 
 		/* I removed check for oif == dev_out->oif here.
 		   It was wrong for two reasons:
@@ -2409,6 +2407,8 @@ static int ip_route_output_slow(struct r
 
 		if (oldflp->oif == 0
 		    && (MULTICAST(oldflp->fl4_dst) || oldflp->fl4_dst == htonl(0xFFFFFFFF))) {
+			if (dev_out == NULL)
+				goto out;
 			/* Special hack: user can direct multicasts
 			   and limited broadcast via necessary interface
 			   without fiddling with IP_MULTICAST_IF or IP_PKTINFO.
@@ -2427,9 +2427,11 @@ static int ip_route_output_slow(struct r
 			fl.oif = dev_out->ifindex;
 			goto make_route;
 		}
-		if (dev_out)
+		if (dev_out) {
 			dev_put(dev_out);
-		dev_out = NULL;
+			dev_out = NULL;
+		} else if (!(oldflp->flags & FLOWI_FLAG_ANYSRC))
+			goto out;
 	}
 
 

	Or we can go further and to avoid ip_dev_find? For me, this
second variant is preferred because calling ip_dev_find() is useless for
FLOWI_FLAG_ANYSRC.

@@ -2394,11 +2394,6 @@ static int ip_route_output_slow(struct r
 		    ZERONET(oldflp->fl4_src))
 			goto out;
 
-		/* It is equivalent to inet_addr_type(saddr) == RTN_LOCAL */
-		dev_out = ip_dev_find(oldflp->fl4_src);
-		if (dev_out == NULL)
-			goto out;
-
 		/* I removed check for oif == dev_out->oif here.
 		   It was wrong for two reasons:
 		   1. ip_dev_find(saddr) can return wrong iface, if saddr is
@@ -2409,6 +2404,11 @@ static int ip_route_output_slow(struct r
 
 		if (oldflp->oif == 0
 		    && (MULTICAST(oldflp->fl4_dst) || oldflp->fl4_dst == htonl(0xFFFFFFFF))) {
+			/* It is equivalent to inet_addr_type(saddr) == RTN_LOCAL */
+			dev_out = ip_dev_find(oldflp->fl4_src);
+			if (dev_out == NULL)
+				goto out;
+
 			/* Special hack: user can direct multicasts
 			   and limited broadcast via necessary interface
 			   without fiddling with IP_MULTICAST_IF or IP_PKTINFO.
@@ -2427,9 +2427,14 @@ static int ip_route_output_slow(struct r
 			fl.oif = dev_out->ifindex;
 			goto make_route;
 		}
-		if (dev_out)
+		if (!(oldflp->flags & FLOWI_FLAG_ANYSRC)) {
+			/* It is equivalent to inet_addr_type(saddr) == RTN_LOCAL */
+			dev_out = ip_dev_find(oldflp->fl4_src);
+			if (dev_out == NULL)
+				goto out;
 			dev_put(dev_out);
-		dev_out = NULL;
+			dev_out = NULL;
+		}
 	}
 
 
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists