| Message ID | 20211117090010.125393-1-pbl@bestov.io (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2] ipv4/raw: support binding to nonlocal addresses | expand |
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 <pbl@bestov.io> > Reviewed-by: David Ahern <dsahern@kernel.org> > --- > 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 <pbl@bestov.io> > > Reviewed-by: David Ahern <dsahern@kernel.org> > > --- > > 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!
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;