Add support to inet raw sockets for binding to nonlocal addresses
through the IP_FREEBIND and IP_TRANSPARENT socket options, as well as
the ipv4.ip_nonlocal_bind kernel parameter.
Signed-off-by: Riccardo Paolo Bestetti <[email protected]>
---
net/ipv4/raw.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 50a73178d63a..734c0332b54b 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -717,6 +717,7 @@ static int raw_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
{
struct inet_sock *inet = inet_sk(sk);
struct sockaddr_in *addr = (struct sockaddr_in *) uaddr;
+ struct net *net = sock_net(sk);
u32 tb_id = RT_TABLE_LOCAL;
int ret = -EINVAL;
int chk_addr_ret;
@@ -732,7 +733,8 @@ static int raw_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
tb_id);
ret = -EADDRNOTAVAIL;
- if (addr->sin_addr.s_addr && chk_addr_ret != RTN_LOCAL &&
+ if (!inet_can_nonlocal_bind(net, inet) &&
+ addr->sin_addr.s_addr && chk_addr_ret != RTN_LOCAL &&
chk_addr_ret != RTN_MULTICAST && chk_addr_ret != RTN_BROADCAST)
goto out;
inet->inet_rcv_saddr = inet->inet_saddr = addr->sin_addr.s_addr;
--
2.31.0
On 3/20/21 6:20 PM, Riccardo Paolo Bestetti wrote:
> diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
> index 50a73178d63a..734c0332b54b 100644
> --- a/net/ipv4/raw.c
> +++ b/net/ipv4/raw.c
> @@ -717,6 +717,7 @@ static int raw_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
> {
> struct inet_sock *inet = inet_sk(sk);
> struct sockaddr_in *addr = (struct sockaddr_in *) uaddr;
> + struct net *net = sock_net(sk);
> u32 tb_id = RT_TABLE_LOCAL;
> int ret = -EINVAL;
> int chk_addr_ret;
> @@ -732,7 +733,8 @@ static int raw_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
> tb_id);
>
> ret = -EADDRNOTAVAIL;
> - if (addr->sin_addr.s_addr && chk_addr_ret != RTN_LOCAL &&
> + if (!inet_can_nonlocal_bind(net, inet) &&
> + addr->sin_addr.s_addr && chk_addr_ret != RTN_LOCAL &&
> chk_addr_ret != RTN_MULTICAST && chk_addr_ret != RTN_BROADCAST)
> goto out;
> inet->inet_rcv_saddr = inet->inet_saddr = addr->sin_addr.s_addr;
>
Please add test cases to ipv4_addr_bind and ipv6_addr_bind in
tools/testing/selftests/net/fcnal-test.sh. The latter will verify if
IPv6 works the same or needs a change.
Also, this check duplicates the ones in __inet_bind and __inet6_bind; it
would be good to use an inline helper to reduce the duplication.
Add support to inet v4 raw sockets for binding to nonlocal addresses
through the IP_FREEBIND and IP_TRANSPARENT socket options, as well as
the ipv4.ip_nonlocal_bind kernel parameter.
Add helper function to inet_sock.h to check for bind address validity on
the base of the address type and whether nonlocal address are enabled
for the socket via any of the sockopts/sysctl, deduplicating checks in
ipv4/ping.c, ipv4/af_inet.c, ipv6/af_inet6.c (for mapped v4->v6
addresses), and ipv4/raw.c.
Add test cases with IP[V6]_FREEBIND verifying that both v4 and v6 raw
sockets support binding to nonlocal addresses after the change. Add
necessary support for the test cases to nettest.
Signed-off-by: Riccardo Paolo Bestetti <[email protected]>
---
Responding to review by David Ahern (21 March 2021),
Thank you for your review.
> Please add test cases to ipv4_addr_bind and ipv6_addr_bind in
> tools/testing/selftests/net/fcnal-test.sh. The latter will verify if
> IPv6 works the same or needs a change.
I have added the tests for both v4 and v6. IPv6 raw sockets already
supported the functionality (under the IPV6_* sockopts), and the (new)
related tests pass, confirming this.
I have not added negative tests (i.e. checking that the same addresses
/fail/ to bind without the necessary flags) because I haven't seen such
tests for other features. If you feel that's needed, I can look into
it.
> Also, this check duplicates the ones in __inet_bind and __inet6_bind; it
> would be good to use an inline helper to reduce the duplication.
Done. The same check was also duplicated in net/ipv4/ping.c, as
detailed in the commit message. I have also deduplicated that, if it
should have been left alone I'll quickly fire up a v3 and revert that.
Sorry for the delay with v2, had a busy year.
include/net/inet_sock.h | 12 +++++++
net/ipv4/af_inet.c | 7 ++--
net/ipv4/ping.c | 14 +++-----
net/ipv4/raw.c | 13 ++++----
net/ipv6/af_inet6.c | 7 ++--
tools/testing/selftests/net/fcnal-test.sh | 40 +++++++++++++++++++++++
tools/testing/selftests/net/nettest.c | 33 ++++++++++++++++++-
7 files changed, 100 insertions(+), 26 deletions(-)
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index 89163ef8cf4b..13b05d116f6b 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -373,4 +373,16 @@ static inline bool inet_can_nonlocal_bind(struct net *net,
inet->freebind || inet->transparent;
}
+static inline bool inet_addr_valid_or_nonlocal(struct net *net,
+ struct inet_sock *inet,
+ __be32 addr,
+ int addr_type)
+{
+ return inet_can_nonlocal_bind(net, inet) ||
+ addr == htonl(INADDR_ANY) ||
+ addr_type == RTN_LOCAL ||
+ addr_type == RTN_MULTICAST ||
+ addr_type == RTN_BROADCAST;
+}
+
#endif /* _INET_SOCK_H */
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 1d816a5fd3eb..fb5cf3623e03 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -492,11 +492,8 @@ int __inet_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len,
* is temporarily down)
*/
err = -EADDRNOTAVAIL;
- if (!inet_can_nonlocal_bind(net, inet) &&
- addr->sin_addr.s_addr != htonl(INADDR_ANY) &&
- chk_addr_ret != RTN_LOCAL &&
- chk_addr_ret != RTN_MULTICAST &&
- chk_addr_ret != RTN_BROADCAST)
+ if (!inet_addr_valid_or_nonlocal(net, inet, addr->sin_addr.s_addr,
+ chk_addr_ret))
goto out;
snum = ntohs(addr->sin_port);
diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index 1e44a43acfe2..e540b0dcf085 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -311,15 +311,11 @@ 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))
- chk_addr_ret = RTN_LOCAL;
- else
- chk_addr_ret = inet_addr_type(net, addr->sin_addr.s_addr);
-
- if ((!inet_can_nonlocal_bind(net, isk) &&
- chk_addr_ret != RTN_LOCAL) ||
- chk_addr_ret == RTN_MULTICAST ||
- chk_addr_ret == RTN_BROADCAST)
+ chk_addr_ret = inet_addr_type(net, addr->sin_addr.s_addr);
+
+ if (!inet_addr_valid_or_nonlocal(net, inet_sk(sk),
+ addr->sin_addr.s_addr,
+ chk_addr_ret))
return -EADDRNOTAVAIL;
#if IS_ENABLED(CONFIG_IPV6)
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index bb446e60cf58..fa60517372b5 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -717,6 +717,7 @@ static int raw_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
{
struct inet_sock *inet = inet_sk(sk);
struct sockaddr_in *addr = (struct sockaddr_in *) uaddr;
+ struct net *net = sock_net(sk);
u32 tb_id = RT_TABLE_LOCAL;
int ret = -EINVAL;
int chk_addr_ret;
@@ -725,16 +726,16 @@ static int raw_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
goto out;
if (sk->sk_bound_dev_if)
- tb_id = l3mdev_fib_table_by_index(sock_net(sk),
- sk->sk_bound_dev_if) ? : tb_id;
+ tb_id = l3mdev_fib_table_by_index(net,
+ sk->sk_bound_dev_if) ? : tb_id;
- chk_addr_ret = inet_addr_type_table(sock_net(sk), addr->sin_addr.s_addr,
- tb_id);
+ chk_addr_ret = inet_addr_type_table(net, addr->sin_addr.s_addr, tb_id);
ret = -EADDRNOTAVAIL;
- if (addr->sin_addr.s_addr && chk_addr_ret != RTN_LOCAL &&
- chk_addr_ret != RTN_MULTICAST && chk_addr_ret != RTN_BROADCAST)
+ if (!inet_addr_valid_or_nonlocal(net, inet, addr->sin_addr.s_addr,
+ chk_addr_ret))
goto out;
+
inet->inet_rcv_saddr = inet->inet_saddr = addr->sin_addr.s_addr;
if (chk_addr_ret == RTN_MULTICAST || chk_addr_ret == RTN_BROADCAST)
inet->inet_saddr = 0; /* Use device */
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index b5878bb8e419..0c557edbbd20 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -337,11 +337,8 @@ static int __inet6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len,
chk_addr_ret = inet_addr_type_dev_table(net, dev, v4addr);
rcu_read_unlock();
- if (!inet_can_nonlocal_bind(net, inet) &&
- v4addr != htonl(INADDR_ANY) &&
- chk_addr_ret != RTN_LOCAL &&
- chk_addr_ret != RTN_MULTICAST &&
- chk_addr_ret != RTN_BROADCAST) {
+ if (!inet_addr_valid_or_nonlocal(net, inet, v4addr,
+ chk_addr_ret)) {
err = -EADDRNOTAVAIL;
goto out;
}
diff --git a/tools/testing/selftests/net/fcnal-test.sh b/tools/testing/selftests/net/fcnal-test.sh
index 3313566ce906..7caa4f0e067d 100755
--- a/tools/testing/selftests/net/fcnal-test.sh
+++ b/tools/testing/selftests/net/fcnal-test.sh
@@ -66,6 +66,10 @@ NSB_LO_IP=172.16.2.2
NSA_LO_IP6=2001:db8:2::1
NSB_LO_IP6=2001:db8:2::2
+# non-local addresses for freebind tests
+NL_IP=172.17.1.1
+NL_IP6=2001:db8:4::1
+
MD5_PW=abc123
MD5_WRONG_PW=abc1234
@@ -316,6 +320,9 @@ addr2str()
${NSB_LO_IP6}) echo "ns-B loopback IPv6";;
${NSB_LINKIP6}|${NSB_LINKIP6}%*) echo "ns-B IPv6 LLA";;
+ ${NL_IP}) echo "nonlocal IP";;
+ ${NL_IP6}) echo "nonlocal IPv6";;
+
${VRF_IP}) echo "VRF IP";;
${VRF_IP6}) echo "VRF IPv6";;
@@ -1767,6 +1774,14 @@ ipv4_addr_bind_novrf()
log_test_addr ${a} $? 0 "Raw socket bind to local address after device bind"
done
+ #
+ # raw socket with nonlocal bind
+ #
+ a=${NL_IP}
+ log_start
+ 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"
+
#
# tcp sockets
#
@@ -1815,6 +1830,14 @@ ipv4_addr_bind_vrf()
run_cmd nettest -s -R -P icmp -l ${a} -I ${VRF} -b
log_test_addr ${a} $? 1 "Raw socket bind to out of scope address after VRF bind"
+ #
+ # raw socket with nonlocal bind
+ #
+ a=${NL_IP}
+ log_start
+ 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"
+
#
# tcp sockets
#
@@ -1965,6 +1988,7 @@ ipv4_rt()
a=${NSA_IP}
log_start
+
run_cmd nettest ${varg} -s &
sleep 1
run_cmd nettest ${varg} -d ${NSA_DEV} -r ${a} &
@@ -3402,6 +3426,14 @@ ipv6_addr_bind_novrf()
log_test_addr ${a} $? 0 "Raw socket bind to local address after device bind"
done
+ #
+ # raw socket with nonlocal bind
+ #
+ a=${NL_IP6}
+ log_start
+ run_cmd nettest -6 -s -R -P icmp -f -l ${a} -I ${NSA_DEV} -b
+ log_test_addr ${a} $? 0 "Raw socket bind to nonlocal address"
+
#
# tcp sockets
#
@@ -3443,6 +3475,14 @@ ipv6_addr_bind_vrf()
run_cmd nettest -6 -s -R -P ipv6-icmp -l ${a} -I ${VRF} -b
log_test_addr ${a} $? 1 "Raw socket bind to invalid local address after vrf bind"
+ #
+ # raw socket with nonlocal bind
+ #
+ a=${NL_IP6}
+ log_start
+ run_cmd nettest -6 -s -R -P icmp -f -l ${a} -I ${VRF} -b
+ log_test_addr ${a} $? 0 "Raw socket bind to nonlocal address after VRF bind"
+
#
# tcp sockets
#
diff --git a/tools/testing/selftests/net/nettest.c b/tools/testing/selftests/net/nettest.c
index b599003eb5ba..d9a6fd2cd9d3 100644
--- a/tools/testing/selftests/net/nettest.c
+++ b/tools/testing/selftests/net/nettest.c
@@ -85,6 +85,7 @@ struct sock_args {
int version; /* AF_INET/AF_INET6 */
int use_setsockopt;
+ int use_freebind;
int use_cmsg;
const char *dev;
const char *server_dev;
@@ -514,6 +515,29 @@ static int set_membership(int sd, uint32_t grp, uint32_t addr, int ifindex)
return 0;
}
+static int set_freebind(int sd, int version)
+{
+ unsigned int one = 1;
+ int rc = 0;
+
+ switch (version) {
+ case AF_INET:
+ if (setsockopt(sd, SOL_IP, IP_FREEBIND, &one, sizeof(one))) {
+ log_err_errno("setsockopt(IP_FREEBIND)");
+ rc = -1;
+ }
+ break;
+ case AF_INET6:
+ if (setsockopt(sd, SOL_IPV6, IPV6_FREEBIND, &one, sizeof(one))) {
+ log_err_errno("setsockopt(IPV6_FREEBIND");
+ rc = -1;
+ }
+ break;
+ }
+
+ return rc;
+}
+
static int set_broadcast(int sd)
{
unsigned int one = 1;
@@ -1419,6 +1443,9 @@ static int lsock_init(struct sock_args *args)
set_unicast_if(sd, args->ifindex, args->version))
goto err;
+ if (args->use_freebind && set_freebind(sd, args->version))
+ goto err;
+
if (bind_socket(sd, args))
goto err;
@@ -1827,7 +1854,7 @@ static int ipc_parent(int cpid, int fd, struct sock_args *args)
return client_status;
}
-#define GETOPT_STR "sr:l:c:p:t:g:P:DRn:M:X:m:d:I:BN:O:SCi6xL:0:1:2:3:Fbq"
+#define GETOPT_STR "sr:l:c:p:t:g:P:DRn:M:X:m:d:I:BN:O:SCi6xL:0:1:2:3:Fbqf"
#define OPT_FORCE_BIND_KEY_IFINDEX 1001
#define OPT_NO_BIND_KEY_IFINDEX 1002
@@ -1864,6 +1891,7 @@ static void print_usage(char *prog)
" -I dev bind socket to given device name - server mode\n"
" -S use setsockopt (IP_UNICAST_IF or IP_MULTICAST_IF)\n"
" to set device binding\n"
+ " -f bind socket with the IP[V6]_FREEBIND option\n"
" -C use cmsg and IP_PKTINFO to specify device binding\n"
"\n"
" -L len send random message of given length\n"
@@ -1999,6 +2027,9 @@ int main(int argc, char *argv[])
case 'S':
args.use_setsockopt = 1;
break;
+ case 'f':
+ args.use_freebind = 1;
+ break;
case 'C':
args.use_cmsg = 1;
break;
--
2.33.1
On 11/2/21 8:19 AM, Riccardo Paolo Bestetti wrote:
> Add support to inet v4 raw sockets for binding to nonlocal addresses
> through the IP_FREEBIND and IP_TRANSPARENT socket options, as well as
> the ipv4.ip_nonlocal_bind kernel parameter.
>
> Add helper function to inet_sock.h to check for bind address validity on
> the base of the address type and whether nonlocal address are enabled
> for the socket via any of the sockopts/sysctl, deduplicating checks in
> ipv4/ping.c, ipv4/af_inet.c, ipv6/af_inet6.c (for mapped v4->v6
> addresses), and ipv4/raw.c.
>
> Add test cases with IP[V6]_FREEBIND verifying that both v4 and v6 raw
> sockets support binding to nonlocal addresses after the change. Add
> necessary support for the test cases to nettest.
>
> Signed-off-by: Riccardo Paolo Bestetti <[email protected]>
> ---
Reviewed-by: David Ahern <[email protected]>
On Tue, 2 Nov 2021 15:19:21 +0100 Riccardo Paolo Bestetti wrote:
> Add support to inet v4 raw sockets for binding to nonlocal addresses
> through the IP_FREEBIND and IP_TRANSPARENT socket options, as well as
> the ipv4.ip_nonlocal_bind kernel parameter.
>
> Add helper function to inet_sock.h to check for bind address validity on
> the base of the address type and whether nonlocal address are enabled
> for the socket via any of the sockopts/sysctl, deduplicating checks in
> ipv4/ping.c, ipv4/af_inet.c, ipv6/af_inet6.c (for mapped v4->v6
> addresses), and ipv4/raw.c.
>
> Add test cases with IP[V6]_FREEBIND verifying that both v4 and v6 raw
> sockets support binding to nonlocal addresses after the change. Add
> necessary support for the test cases to nettest.
>
> Signed-off-by: Riccardo Paolo Bestetti <[email protected]>
Thanks for the patch, please keep Dave's review tag and repost
in ~2 weeks.
# Form letter - net-next is closed
We have already sent the networking pull request for 5.16
and therefore net-next is closed for new drivers, features,
code refactoring and optimizations. We are currently accepting
bug fixes only.
Please repost when net-next reopens after 5.15-rc1 is cut.
Look out for the announcement on the mailing list or check:
http://vger.kernel.org/~davem/net-next.html
On Mon, 15 Nov 2021 00:09:44 +0100 Riccardo Paolo Bestetti wrote:
> Add support to inet v4 raw sockets for binding to nonlocal addresses
> through the IP_FREEBIND and IP_TRANSPARENT socket options, as well as
> the ipv4.ip_nonlocal_bind kernel parameter.
FWIW this patch did not make it to patchwork or any of the mailing
lists. Not immediately obvious why. Can you try re-sending?
Add support to inet v4 raw sockets for binding to nonlocal addresses
through the IP_FREEBIND and IP_TRANSPARENT socket options, as well as
the ipv4.ip_nonlocal_bind kernel parameter.
Add helper function to inet_sock.h to check for bind address validity on
the base of the address type and whether nonlocal address are enabled
for the socket via any of the sockopts/sysctl, deduplicating checks in
ipv4/ping.c, ipv4/af_inet.c, ipv6/af_inet6.c (for mapped v4->v6
addresses), and ipv4/raw.c.
Add test cases with IP[V6]_FREEBIND verifying that both v4 and v6 raw
sockets support binding to nonlocal addresses after the change. Add
necessary support for the test cases to nettest.
Signed-off-by: Riccardo Paolo Bestetti <[email protected]>
Reviewed-by: David Ahern <[email protected]>
---
20211117: resending this, as Patchwork didn't pick it up last time
Responding to review by David Ahern (21 March 2021),
Thank you for your review.
> Please add test cases to ipv4_addr_bind and ipv6_addr_bind in
> tools/testing/selftests/net/fcnal-test.sh. The latter will verify if
> IPv6 works the same or needs a change.
I have added the tests for both v4 and v6. IPv6 raw sockets already
supported the functionality (under the IPV6_* sockopts), and the (new)
related tests pass, confirming this.
I have not added negative tests (i.e. checking that the same addresses
/fail/ to bind without the necessary flags) because I haven't seen such
tests for other features. If you feel that's needed, I can look into
it.
> Also, this check duplicates the ones in __inet_bind and __inet6_bind; it
> would be good to use an inline helper to reduce the duplication.
Done. The same check was also duplicated in net/ipv4/ping.c, as
detailed in the commit message. I have also deduplicated that, if it
should have been left alone I'll quickly fire up a v3 and revert that.
Sorry for the delay with v2, had a busy year.
include/net/inet_sock.h | 12 +++++++
net/ipv4/af_inet.c | 7 ++--
net/ipv4/ping.c | 14 +++-----
net/ipv4/raw.c | 13 ++++----
net/ipv6/af_inet6.c | 7 ++--
tools/testing/selftests/net/fcnal-test.sh | 40 +++++++++++++++++++++++
tools/testing/selftests/net/nettest.c | 33 ++++++++++++++++++-
7 files changed, 100 insertions(+), 26 deletions(-)
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index 89163ef8cf4b..13b05d116f6b 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -373,4 +373,16 @@ static inline bool inet_can_nonlocal_bind(struct net *net,
inet->freebind || inet->transparent;
}
+static inline bool inet_addr_valid_or_nonlocal(struct net *net,
+ struct inet_sock *inet,
+ __be32 addr,
+ int addr_type)
+{
+ return inet_can_nonlocal_bind(net, inet) ||
+ addr == htonl(INADDR_ANY) ||
+ addr_type == RTN_LOCAL ||
+ addr_type == RTN_MULTICAST ||
+ addr_type == RTN_BROADCAST;
+}
+
#endif /* _INET_SOCK_H */
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 1d816a5fd3eb..fb5cf3623e03 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -492,11 +492,8 @@ int __inet_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len,
* is temporarily down)
*/
err = -EADDRNOTAVAIL;
- if (!inet_can_nonlocal_bind(net, inet) &&
- addr->sin_addr.s_addr != htonl(INADDR_ANY) &&
- chk_addr_ret != RTN_LOCAL &&
- chk_addr_ret != RTN_MULTICAST &&
- chk_addr_ret != RTN_BROADCAST)
+ if (!inet_addr_valid_or_nonlocal(net, inet, addr->sin_addr.s_addr,
+ chk_addr_ret))
goto out;
snum = ntohs(addr->sin_port);
diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index 1e44a43acfe2..e540b0dcf085 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -311,15 +311,11 @@ 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))
- chk_addr_ret = RTN_LOCAL;
- else
- chk_addr_ret = inet_addr_type(net, addr->sin_addr.s_addr);
-
- if ((!inet_can_nonlocal_bind(net, isk) &&
- chk_addr_ret != RTN_LOCAL) ||
- chk_addr_ret == RTN_MULTICAST ||
- chk_addr_ret == RTN_BROADCAST)
+ chk_addr_ret = inet_addr_type(net, addr->sin_addr.s_addr);
+
+ if (!inet_addr_valid_or_nonlocal(net, inet_sk(sk),
+ addr->sin_addr.s_addr,
+ chk_addr_ret))
return -EADDRNOTAVAIL;
#if IS_ENABLED(CONFIG_IPV6)
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index bb446e60cf58..fa60517372b5 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -717,6 +717,7 @@ static int raw_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
{
struct inet_sock *inet = inet_sk(sk);
struct sockaddr_in *addr = (struct sockaddr_in *) uaddr;
+ struct net *net = sock_net(sk);
u32 tb_id = RT_TABLE_LOCAL;
int ret = -EINVAL;
int chk_addr_ret;
@@ -725,16 +726,16 @@ static int raw_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
goto out;
if (sk->sk_bound_dev_if)
- tb_id = l3mdev_fib_table_by_index(sock_net(sk),
- sk->sk_bound_dev_if) ? : tb_id;
+ tb_id = l3mdev_fib_table_by_index(net,
+ sk->sk_bound_dev_if) ? : tb_id;
- chk_addr_ret = inet_addr_type_table(sock_net(sk), addr->sin_addr.s_addr,
- tb_id);
+ chk_addr_ret = inet_addr_type_table(net, addr->sin_addr.s_addr, tb_id);
ret = -EADDRNOTAVAIL;
- if (addr->sin_addr.s_addr && chk_addr_ret != RTN_LOCAL &&
- chk_addr_ret != RTN_MULTICAST && chk_addr_ret != RTN_BROADCAST)
+ if (!inet_addr_valid_or_nonlocal(net, inet, addr->sin_addr.s_addr,
+ chk_addr_ret))
goto out;
+
inet->inet_rcv_saddr = inet->inet_saddr = addr->sin_addr.s_addr;
if (chk_addr_ret == RTN_MULTICAST || chk_addr_ret == RTN_BROADCAST)
inet->inet_saddr = 0; /* Use device */
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index b5878bb8e419..0c557edbbd20 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -337,11 +337,8 @@ static int __inet6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len,
chk_addr_ret = inet_addr_type_dev_table(net, dev, v4addr);
rcu_read_unlock();
- if (!inet_can_nonlocal_bind(net, inet) &&
- v4addr != htonl(INADDR_ANY) &&
- chk_addr_ret != RTN_LOCAL &&
- chk_addr_ret != RTN_MULTICAST &&
- chk_addr_ret != RTN_BROADCAST) {
+ if (!inet_addr_valid_or_nonlocal(net, inet, v4addr,
+ chk_addr_ret)) {
err = -EADDRNOTAVAIL;
goto out;
}
diff --git a/tools/testing/selftests/net/fcnal-test.sh b/tools/testing/selftests/net/fcnal-test.sh
index 3313566ce906..7caa4f0e067d 100755
--- a/tools/testing/selftests/net/fcnal-test.sh
+++ b/tools/testing/selftests/net/fcnal-test.sh
@@ -66,6 +66,10 @@ NSB_LO_IP=172.16.2.2
NSA_LO_IP6=2001:db8:2::1
NSB_LO_IP6=2001:db8:2::2
+# non-local addresses for freebind tests
+NL_IP=172.17.1.1
+NL_IP6=2001:db8:4::1
+
MD5_PW=abc123
MD5_WRONG_PW=abc1234
@@ -316,6 +320,9 @@ addr2str()
${NSB_LO_IP6}) echo "ns-B loopback IPv6";;
${NSB_LINKIP6}|${NSB_LINKIP6}%*) echo "ns-B IPv6 LLA";;
+ ${NL_IP}) echo "nonlocal IP";;
+ ${NL_IP6}) echo "nonlocal IPv6";;
+
${VRF_IP}) echo "VRF IP";;
${VRF_IP6}) echo "VRF IPv6";;
@@ -1767,6 +1774,14 @@ ipv4_addr_bind_novrf()
log_test_addr ${a} $? 0 "Raw socket bind to local address after device bind"
done
+ #
+ # raw socket with nonlocal bind
+ #
+ a=${NL_IP}
+ log_start
+ 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"
+
#
# tcp sockets
#
@@ -1815,6 +1830,14 @@ ipv4_addr_bind_vrf()
run_cmd nettest -s -R -P icmp -l ${a} -I ${VRF} -b
log_test_addr ${a} $? 1 "Raw socket bind to out of scope address after VRF bind"
+ #
+ # raw socket with nonlocal bind
+ #
+ a=${NL_IP}
+ log_start
+ 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"
+
#
# tcp sockets
#
@@ -1965,6 +1988,7 @@ ipv4_rt()
a=${NSA_IP}
log_start
+
run_cmd nettest ${varg} -s &
sleep 1
run_cmd nettest ${varg} -d ${NSA_DEV} -r ${a} &
@@ -3402,6 +3426,14 @@ ipv6_addr_bind_novrf()
log_test_addr ${a} $? 0 "Raw socket bind to local address after device bind"
done
+ #
+ # raw socket with nonlocal bind
+ #
+ a=${NL_IP6}
+ log_start
+ run_cmd nettest -6 -s -R -P icmp -f -l ${a} -I ${NSA_DEV} -b
+ log_test_addr ${a} $? 0 "Raw socket bind to nonlocal address"
+
#
# tcp sockets
#
@@ -3443,6 +3475,14 @@ ipv6_addr_bind_vrf()
run_cmd nettest -6 -s -R -P ipv6-icmp -l ${a} -I ${VRF} -b
log_test_addr ${a} $? 1 "Raw socket bind to invalid local address after vrf bind"
+ #
+ # raw socket with nonlocal bind
+ #
+ a=${NL_IP6}
+ log_start
+ run_cmd nettest -6 -s -R -P icmp -f -l ${a} -I ${VRF} -b
+ log_test_addr ${a} $? 0 "Raw socket bind to nonlocal address after VRF bind"
+
#
# tcp sockets
#
diff --git a/tools/testing/selftests/net/nettest.c b/tools/testing/selftests/net/nettest.c
index b599003eb5ba..d9a6fd2cd9d3 100644
--- a/tools/testing/selftests/net/nettest.c
+++ b/tools/testing/selftests/net/nettest.c
@@ -85,6 +85,7 @@ struct sock_args {
int version; /* AF_INET/AF_INET6 */
int use_setsockopt;
+ int use_freebind;
int use_cmsg;
const char *dev;
const char *server_dev;
@@ -514,6 +515,29 @@ static int set_membership(int sd, uint32_t grp, uint32_t addr, int ifindex)
return 0;
}
+static int set_freebind(int sd, int version)
+{
+ unsigned int one = 1;
+ int rc = 0;
+
+ switch (version) {
+ case AF_INET:
+ if (setsockopt(sd, SOL_IP, IP_FREEBIND, &one, sizeof(one))) {
+ log_err_errno("setsockopt(IP_FREEBIND)");
+ rc = -1;
+ }
+ break;
+ case AF_INET6:
+ if (setsockopt(sd, SOL_IPV6, IPV6_FREEBIND, &one, sizeof(one))) {
+ log_err_errno("setsockopt(IPV6_FREEBIND");
+ rc = -1;
+ }
+ break;
+ }
+
+ return rc;
+}
+
static int set_broadcast(int sd)
{
unsigned int one = 1;
@@ -1419,6 +1443,9 @@ static int lsock_init(struct sock_args *args)
set_unicast_if(sd, args->ifindex, args->version))
goto err;
+ if (args->use_freebind && set_freebind(sd, args->version))
+ goto err;
+
if (bind_socket(sd, args))
goto err;
@@ -1827,7 +1854,7 @@ static int ipc_parent(int cpid, int fd, struct sock_args *args)
return client_status;
}
-#define GETOPT_STR "sr:l:c:p:t:g:P:DRn:M:X:m:d:I:BN:O:SCi6xL:0:1:2:3:Fbq"
+#define GETOPT_STR "sr:l:c:p:t:g:P:DRn:M:X:m:d:I:BN:O:SCi6xL:0:1:2:3:Fbqf"
#define OPT_FORCE_BIND_KEY_IFINDEX 1001
#define OPT_NO_BIND_KEY_IFINDEX 1002
@@ -1864,6 +1891,7 @@ static void print_usage(char *prog)
" -I dev bind socket to given device name - server mode\n"
" -S use setsockopt (IP_UNICAST_IF or IP_MULTICAST_IF)\n"
" to set device binding\n"
+ " -f bind socket with the IP[V6]_FREEBIND option\n"
" -C use cmsg and IP_PKTINFO to specify device binding\n"
"\n"
" -L len send random message of given length\n"
@@ -1999,6 +2027,9 @@ int main(int argc, char *argv[])
case 'S':
args.use_setsockopt = 1;
break;
+ case 'f':
+ args.use_freebind = 1;
+ break;
case 'C':
args.use_cmsg = 1;
break;
--
2.33.1
On Wed Nov 17, 2021 at 4:44 AM CET, Jakub Kicinski wrote:
> On Mon, 15 Nov 2021 00:09:44 +0100 Riccardo Paolo Bestetti wrote:
> > Add support to inet v4 raw sockets for binding to nonlocal addresses
> > through the IP_FREEBIND and IP_TRANSPARENT socket options, as well as
> > the ipv4.ip_nonlocal_bind kernel parameter.
>
> FWIW this patch did not make it to patchwork or any of the mailing
> lists. Not immediately obvious why. Can you try re-sending?
It did end up on patchwork[1] last time I sent it. At the time net-next
was closed, so you asked me to send it again when it re-opened. I
probably screwed up the message ID this time around.
As of why it didn't end up on the mailing lists - I have no clue! They
are definitely CC'd.
I re-sent it. Hopefully everything worked this time!
Riccardo
[1] https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
11/17/21 12:00 PM, Riccardo Paolo Bestetti пишет:
> Add support to inet v4 raw sockets for binding to nonlocal addresses
> through the IP_FREEBIND and IP_TRANSPARENT socket options, as well as
> the ipv4.ip_nonlocal_bind kernel parameter.
>
> Add helper function to inet_sock.h to check for bind address validity on
> the base of the address type and whether nonlocal address are enabled
> for the socket via any of the sockopts/sysctl, deduplicating checks in
> ipv4/ping.c, ipv4/af_inet.c, ipv6/af_inet6.c (for mapped v4->v6
> addresses), and ipv4/raw.c.
>
> Add test cases with IP[V6]_FREEBIND verifying that both v4 and v6 raw
> sockets support binding to nonlocal addresses after the change. Add
> necessary support for the test cases to nettest.
>
> Signed-off-by: Riccardo Paolo Bestetti <[email protected]>
> Reviewed-by: David Ahern <[email protected]>
> ---
> 20211117: resending this, as Patchwork didn't pick it up last time
>
> Responding to review by David Ahern (21 March 2021),
>
> Thank you for your review.
>
>> Please add test cases to ipv4_addr_bind and ipv6_addr_bind in
>> tools/testing/selftests/net/fcnal-test.sh. The latter will verify if
>> IPv6 works the same or needs a change.
> I have added the tests for both v4 and v6. IPv6 raw sockets already
> supported the functionality (under the IPV6_* sockopts), and the (new)
> related tests pass, confirming this.
>
> I have not added negative tests (i.e. checking that the same addresses
> /fail/ to bind without the necessary flags) because I haven't seen such
> tests for other features. If you feel that's needed, I can look into
> it.
>
>> Also, this check duplicates the ones in __inet_bind and __inet6_bind; it
>> would be good to use an inline helper to reduce the duplication.
> Done. The same check was also duplicated in net/ipv4/ping.c, as
> detailed in the commit message. I have also deduplicated that, if it
> should have been left alone I'll quickly fire up a v3 and revert that.
>
> Sorry for the delay with v2, had a busy year.
>
>
> include/net/inet_sock.h | 12 +++++++
> net/ipv4/af_inet.c | 7 ++--
> net/ipv4/ping.c | 14 +++-----
> net/ipv4/raw.c | 13 ++++----
> net/ipv6/af_inet6.c | 7 ++--
> tools/testing/selftests/net/fcnal-test.sh | 40 +++++++++++++++++++++++
> tools/testing/selftests/net/nettest.c | 33 ++++++++++++++++++-
> 7 files changed, 100 insertions(+), 26 deletions(-)
>
> diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
> index 89163ef8cf4b..13b05d116f6b 100644
> --- a/include/net/inet_sock.h
> +++ b/include/net/inet_sock.h
> @@ -373,4 +373,16 @@ static inline bool inet_can_nonlocal_bind(struct net *net,
> inet->freebind || inet->transparent;
> }
>
> +static inline bool inet_addr_valid_or_nonlocal(struct net *net,
> + struct inet_sock *inet,
> + __be32 addr,
> + int addr_type)
> +{
> + return inet_can_nonlocal_bind(net, inet) ||
> + addr == htonl(INADDR_ANY) ||
> + addr_type == RTN_LOCAL ||
> + addr_type == RTN_MULTICAST ||
> + addr_type == RTN_BROADCAST;
> +}
> +
> #endif /* _INET_SOCK_H */
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 1d816a5fd3eb..fb5cf3623e03 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -492,11 +492,8 @@ int __inet_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len,
> * is temporarily down)
> */
> err = -EADDRNOTAVAIL;
> - if (!inet_can_nonlocal_bind(net, inet) &&
> - addr->sin_addr.s_addr != htonl(INADDR_ANY) &&
> - chk_addr_ret != RTN_LOCAL &&
> - chk_addr_ret != RTN_MULTICAST &&
> - chk_addr_ret != RTN_BROADCAST)
> + if (!inet_addr_valid_or_nonlocal(net, inet, addr->sin_addr.s_addr,
> + chk_addr_ret))
> goto out;
>
> snum = ntohs(addr->sin_port);
> diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
> index 1e44a43acfe2..e540b0dcf085 100644
> --- a/net/ipv4/ping.c
> +++ b/net/ipv4/ping.c
> @@ -311,15 +311,11 @@ 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))
> - chk_addr_ret = RTN_LOCAL;
> - else
> - chk_addr_ret = inet_addr_type(net, addr->sin_addr.s_addr);
That was done intentionally in commit 0ce779a9f501
> -
> - if ((!inet_can_nonlocal_bind(net, isk) &&
> - chk_addr_ret != RTN_LOCAL) ||
> - chk_addr_ret == RTN_MULTICAST ||
> - chk_addr_ret == RTN_BROADCAST)
> + chk_addr_ret = inet_addr_type(net, addr->sin_addr.s_addr);
> +
> + if (!inet_addr_valid_or_nonlocal(net, inet_sk(sk),
> + addr->sin_addr.s_addr,
> + chk_addr_ret))
> return -EADDRNOTAVAIL;
>
> #if IS_ENABLED(CONFIG_IPV6)
> diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
> index bb446e60cf58..fa60517372b5 100644
> --- a/net/ipv4/raw.c
> +++ b/net/ipv4/raw.c
> @@ -717,6 +717,7 @@ static int raw_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
> {
> struct inet_sock *inet = inet_sk(sk);
> struct sockaddr_in *addr = (struct sockaddr_in *) uaddr;
> + struct net *net = sock_net(sk);
> u32 tb_id = RT_TABLE_LOCAL;
> int ret = -EINVAL;
> int chk_addr_ret;
> @@ -725,16 +726,16 @@ static int raw_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
> goto out;
>
> if (sk->sk_bound_dev_if)
> - tb_id = l3mdev_fib_table_by_index(sock_net(sk),
> - sk->sk_bound_dev_if) ? : tb_id;
> + tb_id = l3mdev_fib_table_by_index(net,
> + sk->sk_bound_dev_if) ? : tb_id;
>
> - chk_addr_ret = inet_addr_type_table(sock_net(sk), addr->sin_addr.s_addr,
> - tb_id);
> + chk_addr_ret = inet_addr_type_table(net, addr->sin_addr.s_addr, tb_id);
>
> ret = -EADDRNOTAVAIL;
> - if (addr->sin_addr.s_addr && chk_addr_ret != RTN_LOCAL &&
> - chk_addr_ret != RTN_MULTICAST && chk_addr_ret != RTN_BROADCAST)
> + if (!inet_addr_valid_or_nonlocal(net, inet, addr->sin_addr.s_addr,
> + chk_addr_ret))
> goto out;
> +
> inet->inet_rcv_saddr = inet->inet_saddr = addr->sin_addr.s_addr;
> if (chk_addr_ret == RTN_MULTICAST || chk_addr_ret == RTN_BROADCAST)
> inet->inet_saddr = 0; /* Use device */
> diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
> index b5878bb8e419..0c557edbbd20 100644
> --- a/net/ipv6/af_inet6.c
> +++ b/net/ipv6/af_inet6.c
> @@ -337,11 +337,8 @@ static int __inet6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len,
> chk_addr_ret = inet_addr_type_dev_table(net, dev, v4addr);
> rcu_read_unlock();
>
> - if (!inet_can_nonlocal_bind(net, inet) &&
> - v4addr != htonl(INADDR_ANY) &&
> - chk_addr_ret != RTN_LOCAL &&
> - chk_addr_ret != RTN_MULTICAST &&
> - chk_addr_ret != RTN_BROADCAST) {
> + if (!inet_addr_valid_or_nonlocal(net, inet, v4addr,
> + chk_addr_ret)) {
> err = -EADDRNOTAVAIL;
> goto out;
> }
> diff --git a/tools/testing/selftests/net/fcnal-test.sh b/tools/testing/selftests/net/fcnal-test.sh
> index 3313566ce906..7caa4f0e067d 100755
> --- a/tools/testing/selftests/net/fcnal-test.sh
> +++ b/tools/testing/selftests/net/fcnal-test.sh
> @@ -66,6 +66,10 @@ NSB_LO_IP=172.16.2.2
> NSA_LO_IP6=2001:db8:2::1
> NSB_LO_IP6=2001:db8:2::2
>
> +# non-local addresses for freebind tests
> +NL_IP=172.17.1.1
> +NL_IP6=2001:db8:4::1
> +
> MD5_PW=abc123
> MD5_WRONG_PW=abc1234
>
> @@ -316,6 +320,9 @@ addr2str()
> ${NSB_LO_IP6}) echo "ns-B loopback IPv6";;
> ${NSB_LINKIP6}|${NSB_LINKIP6}%*) echo "ns-B IPv6 LLA";;
>
> + ${NL_IP}) echo "nonlocal IP";;
> + ${NL_IP6}) echo "nonlocal IPv6";;
> +
> ${VRF_IP}) echo "VRF IP";;
> ${VRF_IP6}) echo "VRF IPv6";;
>
> @@ -1767,6 +1774,14 @@ ipv4_addr_bind_novrf()
> log_test_addr ${a} $? 0 "Raw socket bind to local address after device bind"
> done
>
> + #
> + # raw socket with nonlocal bind
> + #
> + a=${NL_IP}
> + log_start
> + 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"
> +
> #
> # tcp sockets
> #
> @@ -1815,6 +1830,14 @@ ipv4_addr_bind_vrf()
> run_cmd nettest -s -R -P icmp -l ${a} -I ${VRF} -b
> log_test_addr ${a} $? 1 "Raw socket bind to out of scope address after VRF bind"
>
> + #
> + # raw socket with nonlocal bind
> + #
> + a=${NL_IP}
> + log_start
> + 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"
> +
> #
> # tcp sockets
> #
> @@ -1965,6 +1988,7 @@ ipv4_rt()
>
> a=${NSA_IP}
> log_start
> +
> run_cmd nettest ${varg} -s &
> sleep 1
> run_cmd nettest ${varg} -d ${NSA_DEV} -r ${a} &
> @@ -3402,6 +3426,14 @@ ipv6_addr_bind_novrf()
> log_test_addr ${a} $? 0 "Raw socket bind to local address after device bind"
> done
>
> + #
> + # raw socket with nonlocal bind
> + #
> + a=${NL_IP6}
> + log_start
> + run_cmd nettest -6 -s -R -P icmp -f -l ${a} -I ${NSA_DEV} -b
> + log_test_addr ${a} $? 0 "Raw socket bind to nonlocal address"
> +
> #
> # tcp sockets
> #
> @@ -3443,6 +3475,14 @@ ipv6_addr_bind_vrf()
> run_cmd nettest -6 -s -R -P ipv6-icmp -l ${a} -I ${VRF} -b
> log_test_addr ${a} $? 1 "Raw socket bind to invalid local address after vrf bind"
>
> + #
> + # raw socket with nonlocal bind
> + #
> + a=${NL_IP6}
> + log_start
> + run_cmd nettest -6 -s -R -P icmp -f -l ${a} -I ${VRF} -b
> + log_test_addr ${a} $? 0 "Raw socket bind to nonlocal address after VRF bind"
> +
> #
> # tcp sockets
> #
> diff --git a/tools/testing/selftests/net/nettest.c b/tools/testing/selftests/net/nettest.c
> index b599003eb5ba..d9a6fd2cd9d3 100644
> --- a/tools/testing/selftests/net/nettest.c
> +++ b/tools/testing/selftests/net/nettest.c
> @@ -85,6 +85,7 @@ struct sock_args {
> int version; /* AF_INET/AF_INET6 */
>
> int use_setsockopt;
> + int use_freebind;
> int use_cmsg;
> const char *dev;
> const char *server_dev;
> @@ -514,6 +515,29 @@ static int set_membership(int sd, uint32_t grp, uint32_t addr, int ifindex)
> return 0;
> }
>
> +static int set_freebind(int sd, int version)
> +{
> + unsigned int one = 1;
> + int rc = 0;
> +
> + switch (version) {
> + case AF_INET:
> + if (setsockopt(sd, SOL_IP, IP_FREEBIND, &one, sizeof(one))) {
> + log_err_errno("setsockopt(IP_FREEBIND)");
> + rc = -1;
> + }
> + break;
> + case AF_INET6:
> + if (setsockopt(sd, SOL_IPV6, IPV6_FREEBIND, &one, sizeof(one))) {
> + log_err_errno("setsockopt(IPV6_FREEBIND");
> + rc = -1;
> + }
> + break;
> + }
> +
> + return rc;
> +}
> +
> static int set_broadcast(int sd)
> {
> unsigned int one = 1;
> @@ -1419,6 +1443,9 @@ static int lsock_init(struct sock_args *args)
> set_unicast_if(sd, args->ifindex, args->version))
> goto err;
>
> + if (args->use_freebind && set_freebind(sd, args->version))
> + goto err;
> +
> if (bind_socket(sd, args))
> goto err;
>
> @@ -1827,7 +1854,7 @@ static int ipc_parent(int cpid, int fd, struct sock_args *args)
> return client_status;
> }
>
> -#define GETOPT_STR "sr:l:c:p:t:g:P:DRn:M:X:m:d:I:BN:O:SCi6xL:0:1:2:3:Fbq"
> +#define GETOPT_STR "sr:l:c:p:t:g:P:DRn:M:X:m:d:I:BN:O:SCi6xL:0:1:2:3:Fbqf"
> #define OPT_FORCE_BIND_KEY_IFINDEX 1001
> #define OPT_NO_BIND_KEY_IFINDEX 1002
>
> @@ -1864,6 +1891,7 @@ static void print_usage(char *prog)
> " -I dev bind socket to given device name - server mode\n"
> " -S use setsockopt (IP_UNICAST_IF or IP_MULTICAST_IF)\n"
> " to set device binding\n"
> + " -f bind socket with the IP[V6]_FREEBIND option\n"
> " -C use cmsg and IP_PKTINFO to specify device binding\n"
> "\n"
> " -L len send random message of given length\n"
> @@ -1999,6 +2027,9 @@ int main(int argc, char *argv[])
> case 'S':
> args.use_setsockopt = 1;
> break;
> + case 'f':
> + args.use_freebind = 1;
> + break;
> case 'C':
> args.use_cmsg = 1;
> break;
>
On Wed Nov 17, 2021 at 4:14 PM CET, Denis Kirjanov wrote:
>
>
> 11/17/21 12:00 PM, Riccardo Paolo Bestetti пишет:
> > Add support to inet v4 raw sockets for binding to nonlocal addresses
> > through the IP_FREEBIND and IP_TRANSPARENT socket options, as well as
> > the ipv4.ip_nonlocal_bind kernel parameter.
> >
> > Add helper function to inet_sock.h to check for bind address validity on
> > the base of the address type and whether nonlocal address are enabled
> > for the socket via any of the sockopts/sysctl, deduplicating checks in
> > ipv4/ping.c, ipv4/af_inet.c, ipv6/af_inet6.c (for mapped v4->v6
> > addresses), and ipv4/raw.c.
> >
> > Add test cases with IP[V6]_FREEBIND verifying that both v4 and v6 raw
> > sockets support binding to nonlocal addresses after the change. Add
> > necessary support for the test cases to nettest.
> >
> > Signed-off-by: Riccardo Paolo Bestetti <[email protected]>
> > Reviewed-by: David Ahern <[email protected]>
> > ---
> > 20211117: resending this, as Patchwork didn't pick it up last time
> >
> > Responding to review by David Ahern (21 March 2021),
> >
> > Thank you for your review.
> >
> >> Please add test cases to ipv4_addr_bind and ipv6_addr_bind in
> >> tools/testing/selftests/net/fcnal-test.sh. The latter will verify if
> >> IPv6 works the same or needs a change.
> > I have added the tests for both v4 and v6. IPv6 raw sockets already
> > supported the functionality (under the IPV6_* sockopts), and the (new)
> > related tests pass, confirming this.
> >
> > I have not added negative tests (i.e. checking that the same addresses
> > /fail/ to bind without the necessary flags) because I haven't seen such
> > tests for other features. If you feel that's needed, I can look into
> > it.
> >
> >> Also, this check duplicates the ones in __inet_bind and __inet6_bind; it
> >> would be good to use an inline helper to reduce the duplication.
> > Done. The same check was also duplicated in net/ipv4/ping.c, as
> > detailed in the commit message. I have also deduplicated that, if it
> > should have been left alone I'll quickly fire up a v3 and revert that.
> >
> > Sorry for the delay with v2, had a busy year.
> >
> >
> > include/net/inet_sock.h | 12 +++++++
> > net/ipv4/af_inet.c | 7 ++--
> > net/ipv4/ping.c | 14 +++-----
> > net/ipv4/raw.c | 13 ++++----
> > net/ipv6/af_inet6.c | 7 ++--
> > tools/testing/selftests/net/fcnal-test.sh | 40 +++++++++++++++++++++++
> > tools/testing/selftests/net/nettest.c | 33 ++++++++++++++++++-
> > 7 files changed, 100 insertions(+), 26 deletions(-)
> >
> > diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
> > index 89163ef8cf4b..13b05d116f6b 100644
> > --- a/include/net/inet_sock.h
> > +++ b/include/net/inet_sock.h
> > @@ -373,4 +373,16 @@ static inline bool inet_can_nonlocal_bind(struct net *net,
> > inet->freebind || inet->transparent;
> > }
> >
> > +static inline bool inet_addr_valid_or_nonlocal(struct net *net,
> > + struct inet_sock *inet,
> > + __be32 addr,
> > + int addr_type)
> > +{
> > + return inet_can_nonlocal_bind(net, inet) ||
> > + addr == htonl(INADDR_ANY) ||
> > + addr_type == RTN_LOCAL ||
> > + addr_type == RTN_MULTICAST ||
> > + addr_type == RTN_BROADCAST;
> > +}
> > +
> > #endif /* _INET_SOCK_H */
> > diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> > index 1d816a5fd3eb..fb5cf3623e03 100644
> > --- a/net/ipv4/af_inet.c
> > +++ b/net/ipv4/af_inet.c
> > @@ -492,11 +492,8 @@ int __inet_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len,
> > * is temporarily down)
> > */
> > err = -EADDRNOTAVAIL;
> > - if (!inet_can_nonlocal_bind(net, inet) &&
> > - addr->sin_addr.s_addr != htonl(INADDR_ANY) &&
> > - chk_addr_ret != RTN_LOCAL &&
> > - chk_addr_ret != RTN_MULTICAST &&
> > - chk_addr_ret != RTN_BROADCAST)
> > + if (!inet_addr_valid_or_nonlocal(net, inet, addr->sin_addr.s_addr,
> > + chk_addr_ret))
> > goto out;
> >
> > snum = ntohs(addr->sin_port);
> > diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
> > index 1e44a43acfe2..e540b0dcf085 100644
> > --- a/net/ipv4/ping.c
> > +++ b/net/ipv4/ping.c
> > @@ -311,15 +311,11 @@ 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))
> > - chk_addr_ret = RTN_LOCAL;
> > - else
> > - chk_addr_ret = inet_addr_type(net, addr->sin_addr.s_addr);
>
> That was done intentionally in commit 0ce779a9f501
>
Should I remove this from the patch? Is there a particular reason why
this was done in ping.c but not in the other places?
> > -
> > - if ((!inet_can_nonlocal_bind(net, isk) &&
> > - chk_addr_ret != RTN_LOCAL) ||
> > - chk_addr_ret == RTN_MULTICAST ||
> > - chk_addr_ret == RTN_BROADCAST)
> > + chk_addr_ret = inet_addr_type(net, addr->sin_addr.s_addr);
> > +
> > + if (!inet_addr_valid_or_nonlocal(net, inet_sk(sk),
> > + addr->sin_addr.s_addr,
> > + chk_addr_ret))
> > return -EADDRNOTAVAIL;
> >
> > #if IS_ENABLED(CONFIG_IPV6)
> > diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
> > index bb446e60cf58..fa60517372b5 100644
> > --- a/net/ipv4/raw.c
> > +++ b/net/ipv4/raw.c
> > @@ -717,6 +717,7 @@ static int raw_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
> > {
> > struct inet_sock *inet = inet_sk(sk);
> > struct sockaddr_in *addr = (struct sockaddr_in *) uaddr;
> > + struct net *net = sock_net(sk);
> > u32 tb_id = RT_TABLE_LOCAL;
> > int ret = -EINVAL;
> > int chk_addr_ret;
> > @@ -725,16 +726,16 @@ static int raw_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
> > goto out;
> >
> > if (sk->sk_bound_dev_if)
> > - tb_id = l3mdev_fib_table_by_index(sock_net(sk),
> > - sk->sk_bound_dev_if) ? : tb_id;
> > + tb_id = l3mdev_fib_table_by_index(net,
> > + sk->sk_bound_dev_if) ? : tb_id;
> >
> > - chk_addr_ret = inet_addr_type_table(sock_net(sk), addr->sin_addr.s_addr,
> > - tb_id);
> > + chk_addr_ret = inet_addr_type_table(net, addr->sin_addr.s_addr, tb_id);
> >
> > ret = -EADDRNOTAVAIL;
> > - if (addr->sin_addr.s_addr && chk_addr_ret != RTN_LOCAL &&
> > - chk_addr_ret != RTN_MULTICAST && chk_addr_ret != RTN_BROADCAST)
> > + if (!inet_addr_valid_or_nonlocal(net, inet, addr->sin_addr.s_addr,
> > + chk_addr_ret))
> > goto out;
> > +
> > inet->inet_rcv_saddr = inet->inet_saddr = addr->sin_addr.s_addr;
> > if (chk_addr_ret == RTN_MULTICAST || chk_addr_ret == RTN_BROADCAST)
> > inet->inet_saddr = 0; /* Use device */
> > diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
> > index b5878bb8e419..0c557edbbd20 100644
> > --- a/net/ipv6/af_inet6.c
> > +++ b/net/ipv6/af_inet6.c
> > @@ -337,11 +337,8 @@ static int __inet6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len,
> > chk_addr_ret = inet_addr_type_dev_table(net, dev, v4addr);
> > rcu_read_unlock();
> >
> > - if (!inet_can_nonlocal_bind(net, inet) &&
> > - v4addr != htonl(INADDR_ANY) &&
> > - chk_addr_ret != RTN_LOCAL &&
> > - chk_addr_ret != RTN_MULTICAST &&
> > - chk_addr_ret != RTN_BROADCAST) {
> > + if (!inet_addr_valid_or_nonlocal(net, inet, v4addr,
> > + chk_addr_ret)) {
> > err = -EADDRNOTAVAIL;
> > goto out;
> > }
> > diff --git a/tools/testing/selftests/net/fcnal-test.sh b/tools/testing/selftests/net/fcnal-test.sh
> > index 3313566ce906..7caa4f0e067d 100755
> > --- a/tools/testing/selftests/net/fcnal-test.sh
> > +++ b/tools/testing/selftests/net/fcnal-test.sh
> > @@ -66,6 +66,10 @@ NSB_LO_IP=172.16.2.2
> > NSA_LO_IP6=2001:db8:2::1
> > NSB_LO_IP6=2001:db8:2::2
> >
> > +# non-local addresses for freebind tests
> > +NL_IP=172.17.1.1
> > +NL_IP6=2001:db8:4::1
> > +
> > MD5_PW=abc123
> > MD5_WRONG_PW=abc1234
> >
> > @@ -316,6 +320,9 @@ addr2str()
> > ${NSB_LO_IP6}) echo "ns-B loopback IPv6";;
> > ${NSB_LINKIP6}|${NSB_LINKIP6}%*) echo "ns-B IPv6 LLA";;
> >
> > + ${NL_IP}) echo "nonlocal IP";;
> > + ${NL_IP6}) echo "nonlocal IPv6";;
> > +
> > ${VRF_IP}) echo "VRF IP";;
> > ${VRF_IP6}) echo "VRF IPv6";;
> >
> > @@ -1767,6 +1774,14 @@ ipv4_addr_bind_novrf()
> > log_test_addr ${a} $? 0 "Raw socket bind to local address after device bind"
> > done
> >
> > + #
> > + # raw socket with nonlocal bind
> > + #
> > + a=${NL_IP}
> > + log_start
> > + 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"
> > +
> > #
> > # tcp sockets
> > #
> > @@ -1815,6 +1830,14 @@ ipv4_addr_bind_vrf()
> > run_cmd nettest -s -R -P icmp -l ${a} -I ${VRF} -b
> > log_test_addr ${a} $? 1 "Raw socket bind to out of scope address after VRF bind"
> >
> > + #
> > + # raw socket with nonlocal bind
> > + #
> > + a=${NL_IP}
> > + log_start
> > + 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"
> > +
> > #
> > # tcp sockets
> > #
> > @@ -1965,6 +1988,7 @@ ipv4_rt()
> >
> > a=${NSA_IP}
> > log_start
> > +
> > run_cmd nettest ${varg} -s &
> > sleep 1
> > run_cmd nettest ${varg} -d ${NSA_DEV} -r ${a} &
> > @@ -3402,6 +3426,14 @@ ipv6_addr_bind_novrf()
> > log_test_addr ${a} $? 0 "Raw socket bind to local address after device bind"
> > done
> >
> > + #
> > + # raw socket with nonlocal bind
> > + #
> > + a=${NL_IP6}
> > + log_start
> > + run_cmd nettest -6 -s -R -P icmp -f -l ${a} -I ${NSA_DEV} -b
> > + log_test_addr ${a} $? 0 "Raw socket bind to nonlocal address"
> > +
> > #
> > # tcp sockets
> > #
> > @@ -3443,6 +3475,14 @@ ipv6_addr_bind_vrf()
> > run_cmd nettest -6 -s -R -P ipv6-icmp -l ${a} -I ${VRF} -b
> > log_test_addr ${a} $? 1 "Raw socket bind to invalid local address after vrf bind"
> >
> > + #
> > + # raw socket with nonlocal bind
> > + #
> > + a=${NL_IP6}
> > + log_start
> > + run_cmd nettest -6 -s -R -P icmp -f -l ${a} -I ${VRF} -b
> > + log_test_addr ${a} $? 0 "Raw socket bind to nonlocal address after VRF bind"
> > +
> > #
> > # tcp sockets
> > #
> > diff --git a/tools/testing/selftests/net/nettest.c b/tools/testing/selftests/net/nettest.c
> > index b599003eb5ba..d9a6fd2cd9d3 100644
> > --- a/tools/testing/selftests/net/nettest.c
> > +++ b/tools/testing/selftests/net/nettest.c
> > @@ -85,6 +85,7 @@ struct sock_args {
> > int version; /* AF_INET/AF_INET6 */
> >
> > int use_setsockopt;
> > + int use_freebind;
> > int use_cmsg;
> > const char *dev;
> > const char *server_dev;
> > @@ -514,6 +515,29 @@ static int set_membership(int sd, uint32_t grp, uint32_t addr, int ifindex)
> > return 0;
> > }
> >
> > +static int set_freebind(int sd, int version)
> > +{
> > + unsigned int one = 1;
> > + int rc = 0;
> > +
> > + switch (version) {
> > + case AF_INET:
> > + if (setsockopt(sd, SOL_IP, IP_FREEBIND, &one, sizeof(one))) {
> > + log_err_errno("setsockopt(IP_FREEBIND)");
> > + rc = -1;
> > + }
> > + break;
> > + case AF_INET6:
> > + if (setsockopt(sd, SOL_IPV6, IPV6_FREEBIND, &one, sizeof(one))) {
> > + log_err_errno("setsockopt(IPV6_FREEBIND");
> > + rc = -1;
> > + }
> > + break;
> > + }
> > +
> > + return rc;
> > +}
> > +
> > static int set_broadcast(int sd)
> > {
> > unsigned int one = 1;
> > @@ -1419,6 +1443,9 @@ static int lsock_init(struct sock_args *args)
> > set_unicast_if(sd, args->ifindex, args->version))
> > goto err;
> >
> > + if (args->use_freebind && set_freebind(sd, args->version))
> > + goto err;
> > +
> > if (bind_socket(sd, args))
> > goto err;
> >
> > @@ -1827,7 +1854,7 @@ static int ipc_parent(int cpid, int fd, struct sock_args *args)
> > return client_status;
> > }
> >
> > -#define GETOPT_STR "sr:l:c:p:t:g:P:DRn:M:X:m:d:I:BN:O:SCi6xL:0:1:2:3:Fbq"
> > +#define GETOPT_STR "sr:l:c:p:t:g:P:DRn:M:X:m:d:I:BN:O:SCi6xL:0:1:2:3:Fbqf"
> > #define OPT_FORCE_BIND_KEY_IFINDEX 1001
> > #define OPT_NO_BIND_KEY_IFINDEX 1002
> >
> > @@ -1864,6 +1891,7 @@ static void print_usage(char *prog)
> > " -I dev bind socket to given device name - server mode\n"
> > " -S use setsockopt (IP_UNICAST_IF or IP_MULTICAST_IF)\n"
> > " to set device binding\n"
> > + " -f bind socket with the IP[V6]_FREEBIND option\n"
> > " -C use cmsg and IP_PKTINFO to specify device binding\n"
> > "\n"
> > " -L len send random message of given length\n"
> > @@ -1999,6 +2027,9 @@ int main(int argc, char *argv[])
> > case 'S':
> > args.use_setsockopt = 1;
> > break;
> > + case 'f':
> > + args.use_freebind = 1;
> > + break;
> > case 'C':
> > args.use_cmsg = 1;
> > break;
> >
On 11/17/21 5:14 PM, Riccardo Paolo Bestetti wrote:
>>> diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
>>> index 1e44a43acfe2..e540b0dcf085 100644
>>> --- a/net/ipv4/ping.c
>>> +++ b/net/ipv4/ping.c
>>> @@ -311,15 +311,11 @@ 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))
>>> - chk_addr_ret = RTN_LOCAL;
>>> - else
>>> - chk_addr_ret = inet_addr_type(net, addr->sin_addr.s_addr);
>>
>> That was done intentionally in commit 0ce779a9f501
>>
>
> Should I remove this from the patch? Is there a particular reason why
> this was done in ping.c but not in the other places?
>
exactly. I did not see the big deal about changing it.
On Wed, 17 Nov 2021 20:59:16 -0700 David Ahern wrote:
> On 11/17/21 5:14 PM, Riccardo Paolo Bestetti wrote:
> >> That was done intentionally in commit 0ce779a9f501
> >
> > Should I remove this from the patch? Is there a particular reason why
> > this was done in ping.c but not in the other places?
>
> exactly. I did not see the big deal about changing it.
Applied, thanks!