netdev - Re: [PATCH net-next 6/9] net: Fix up inet_addr

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: <20150811.111457.652499160223100513.davem@davemloft.net>
Date:	Tue, 11 Aug 2015 11:14:57 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	dsa@...ulusnetworks.com
Cc:	netdev@...r.kernel.org, shm@...ulusnetworks.com,
	roopa@...ulusnetworks.com, gospo@...ulusnetworks.com,
	jtoppins@...ulusnetworks.com, nikolay@...ulusnetworks.com,
	ddutt@...ulusnetworks.com, hannes@...essinduktion.org,
	nicolas.dichtel@...nd.com, stephen@...workplumber.org,
	hadi@...atatu.com, ebiederm@...ssion.com, svaidya@...cade.com
Subject: Re: [PATCH net-next 6/9] net: Fix up inet_addr_type checks

From: David Ahern <dsa@...ulusnetworks.com>
Date: Mon, 10 Aug 2015 11:50:33 -0600

> @@ -427,6 +428,7 @@ int inet_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
>  	struct net *net = sock_net(sk);
>  	unsigned short snum;
>  	int chk_addr_ret;
> +	int tb_id = 0;
>  	int err;
>  
>  	/* If the socket has its own bind function then use it. (RAW) */
> @@ -448,7 +450,16 @@ int inet_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
>  			goto out;
>  	}
>  
> -	chk_addr_ret = inet_addr_type(net, addr->sin_addr.s_addr);
> +	if (sk->sk_bound_dev_if) {
> +		struct net_device *dev;
> +
> +		rcu_read_lock();
> +		dev = dev_get_by_index_rcu(net, sk->sk_bound_dev_if);
> +		if (dev)
> +			tb_id = vrf_dev_table_rcu(dev);
> +		rcu_read_unlock();
> +	}
> +	chk_addr_ret = inet_addr_type_table(net, addr->sin_addr.s_addr, tb_id);
>  
>  	/* Not specified by any standard per-se, however it breaks too
>  	 * many applications when removed.  It is unfortunate since
 ...
> diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
> index b11321a8e58d..d84ae0e30369 100644
> --- a/net/ipv4/fib_frontend.c
> +++ b/net/ipv4/fib_frontend.c
> @@ -226,6 +226,9 @@ static inline unsigned int __inet_dev_addr_type(struct net *net,
>  
>  	rcu_read_lock();
>  
> +	if (!tb_id)
> +		tb_id = RT_TABLE_LOCAL;
> +
>  	table = fib_get_table(net, tb_id);

All of this code that quietly translates table ID zero into RT_TABLE_LOCAL is
confusing.

It would be so much easier to understand if the code was structured like:

	int tb_id = RT_TABLE_LOCAL;

	if (doing_vrf_stuff)
		tb_id = foo;
--
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