Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 17 Jun 2022 02:02:13 +0000
Message-Id: <20220617020213.1881452-1-cmllamas@google.com>
Mime-Version: 1.0
Subject: [PATCH net] ipv4: ping: fix bind address validity check
From:   Carlos Llamas <cmllamas@google.com>
To:     "David S . Miller" <davem@davemloft.net>,
        Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
        David Ahern <dsahern@kernel.org>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Riccardo Paolo Bestetti <pbl@bestov.io>
Cc:     kernel-team@android.com, linux-kernel@vger.kernel.org,
        netdev@vger.kernel.org, Carlos Llamas <cmllamas@google.com>,
        Miaohe Lin <linmiaohe@huawei.com>,
        "=?UTF-8?q?Maciej=20=C5=BBenczykowski?=" <maze@google.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Commit 8ff978b8b222 ("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 0ce779a9f501 ("net: Avoid
unnecessary inet_addr_type() call when addr is INADDR_ANY").

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

Fixes: 8ff978b8b222 ("ipv4/raw: support binding to nonlocal addresses")
Cc: Miaohe Lin <linmiaohe@huawei.com>
Cc: Riccardo Paolo Bestetti <pbl@bestov.io>
Reported-by: Maciej =C5=BBenczykowski <maze@google.com>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
---
 net/ipv4/ping.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index 1a43ca73f94d..3c6101def7d6 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -319,12 +319,16 @@ static int ping_check_bind_addr(struct sock *sk, stru=
ct inet_sock *isk,
 		pr_debug("ping_check_bind_addr(sk=3D%p,addr=3D%pI4,port=3D%d)\n",
 			 sk, &addr->sin_addr.s_addr, ntohs(addr->sin_port));
=20
+		if (addr->sin_addr.s_addr =3D=3D htonl(INADDR_ANY))
+			return 0;
+
 		tb_id =3D l3mdev_fib_table_by_index(net, sk->sk_bound_dev_if) ? : tb_id;
 		chk_addr_ret =3D inet_addr_type_table(net, addr->sin_addr.s_addr, tb_id)=
;
=20
-		if (!inet_addr_valid_or_nonlocal(net, inet_sk(sk),
-					         addr->sin_addr.s_addr,
-	                                         chk_addr_ret))
+		if (chk_addr_ret =3D=3D RTN_MULTICAST ||
+		    chk_addr_ret =3D=3D RTN_BROADCAST ||
+		    (chk_addr_ret !=3D RTN_LOCAL &&
+		     !inet_can_nonlocal_bind(net, isk)))
 			return -EADDRNOTAVAIL;
=20
 #if IS_ENABLED(CONFIG_IPV6)
--=20
2.36.1.476.g0c4daa206d-goog