Commit b4a028c4 authored by Riccardo Paolo Bestetti's avatar Riccardo Paolo Bestetti Committed by David S. Miller
Browse files

ipv4: ping: fix bind address validity check 

Commit 8ff978b8 ("ipv4/raw: support binding to nonlocal addresses")
introduced a helper function to fold duplicated validity checks of bind
addresses into inet_addr_valid_or_nonlocal(). However, this caused an
unintended regression in ping_check_bind_addr(), which previously would
reject binding to multicast and broadcast addresses, but now these are
both incorrectly allowed as reported in [1].

This patch restores the original check. A simple reordering is done to
improve readability and make it evident that multicast and broadcast
addresses should not be allowed. Also, add an early exit for INADDR_ANY
which replaces lost behavior added by commit 0ce779a9 ("net: Avoid
unnecessary inet_addr_type() call when addr is INADDR_ANY").

Furthermore, this patch introduces regression selftests to catch these
specific cases.

[1] https://lore.kernel.org/netdev/CANP3RGdkAcDyAZoT1h8Gtuu0saq+eOrrTiWbxnOs+5zn+cpyKg@mail.gmail.com/



Fixes: 8ff978b8 ("ipv4/raw: support binding to nonlocal addresses")
Cc: Miaohe Lin <linmiaohe@huawei.com>
Reported-by: default avatarMaciej Żenczykowski <maze@google.com>
Signed-off-by: default avatarCarlos Llamas <cmllamas@google.com>
Signed-off-by: default avatarRiccardo Paolo Bestetti <pbl@bestov.io>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 2b04495e

net/ipv4/ping.c

+7 −3
Original line number Diff line number Diff line
@@ -319,12 +319,16 @@ 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));



		if (addr->sin_addr.s_addr == htonl(INADDR_ANY))

			return 0;



		tb_id = l3mdev_fib_table_by_index(net, sk->sk_bound_dev_if) ? : tb_id;

		chk_addr_ret = inet_addr_type_table(net, addr->sin_addr.s_addr, tb_id);



		if (!inet_addr_valid_or_nonlocal(net, inet_sk(sk),

					         addr->sin_addr.s_addr,

	                                         chk_addr_ret))

		if (chk_addr_ret == RTN_MULTICAST ||

		    chk_addr_ret == RTN_BROADCAST ||

		    (chk_addr_ret != RTN_LOCAL &&

		     !inet_can_nonlocal_bind(net, isk)))

			return -EADDRNOTAVAIL;



#if IS_ENABLED(CONFIG_IPV6)

tools/testing/selftests/net/fcnal-test.sh

+33 −0
Original line number Diff line number Diff line
@@ -70,6 +70,10 @@ NSB_LO_IP6=2001:db8:2::2 
NL_IP=172.17.1.1

NL_IP6=2001:db8:4::1



# multicast and broadcast addresses

MCAST_IP=224.0.0.1

BCAST_IP=255.255.255.255



MD5_PW=abc123

MD5_WRONG_PW=abc1234
@@ -308,6 +312,9 @@ addr2str() 
	127.0.0.1) echo "loopback";;

	::1) echo "IPv6 loopback";;



	${BCAST_IP}) echo "broadcast";;

	${MCAST_IP}) echo "multicast";;



	${NSA_IP})	echo "ns-A IP";;

	${NSA_IP6})	echo "ns-A IPv6";;

	${NSA_LO_IP})	echo "ns-A loopback IP";;
@@ -1800,6 +1807,19 @@ ipv4_addr_bind_novrf() 
	run_cmd nettest -s -R -P icmp -f -l ${a} -I ${NSA_DEV} -b

	log_test_addr ${a} $? 0 "Raw socket bind to nonlocal address after device bind"



	#

	# check that ICMP sockets cannot bind to broadcast and multicast addresses

	#

	a=${BCAST_IP}

	log_start

	run_cmd nettest -s -R -P icmp -l ${a} -b

	log_test_addr ${a} $? 1 "ICMP socket bind to broadcast address"



	a=${MCAST_IP}

	log_start

	run_cmd nettest -s -R -P icmp -f -l ${a} -b

	log_test_addr ${a} $? 1 "ICMP socket bind to multicast address"



	#

	# tcp sockets

	#
@@ -1857,6 +1877,19 @@ ipv4_addr_bind_vrf() 
	run_cmd nettest -s -R -P icmp -f -l ${a} -I ${VRF} -b

	log_test_addr ${a} $? 0 "Raw socket bind to nonlocal address after VRF bind"



	#

	# check that ICMP sockets cannot bind to broadcast and multicast addresses

	#

	a=${BCAST_IP}

	log_start

	run_cmd nettest -s -R -P icmp -l ${a} -I ${VRF} -b

	log_test_addr ${a} $? 1 "ICMP socket bind to broadcast address after VRF bind"



	a=${MCAST_IP}

	log_start

	run_cmd nettest -s -R -P icmp -f -l ${a} -I ${VRF} -b

	log_test_addr ${a} $? 1 "ICMP socket bind to multicast address after VRF bind"



	#

	# tcp sockets

	#