Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ping: socket: Address family not supported by protocol #293

Closed
lekto opened this issue Sep 2, 2020 · 24 comments
Closed

ping: socket: Address family not supported by protocol #293

lekto opened this issue Sep 2, 2020 · 24 comments
Assignees

Comments

@lekto
Copy link

lekto commented Sep 2, 2020

Hi, I got message "ping: socket: Address family not supported by protocol" when using ping without "-4" after update to s20200821.

lekto@blaszak ~ $ ping github.com
ping: socket: Address family not supported by protocol
PING github.com (140.82.121.3) 56(84) bytes of data.
64 bytes from lb-140-82-121-3-fra.github.com (140.82.121.3): icmp_seq=1 ttl=50 time=19.5 ms
64 bytes from lb-140-82-121-3-fra.github.com (140.82.121.3): icmp_seq=2 ttl=50 time=18.7 ms
64 bytes from lb-140-82-121-3-fra.github.com (140.82.121.3): icmp_seq=3 ttl=50 time=18.3 ms
64 bytes from lb-140-82-121-3-fra.github.com (140.82.121.3): icmp_seq=4 ttl=50 time=17.8 ms
^C
--- github.com ping statistics ---
4 packets transmitted, 4 received, 0% packet loss, time 3003ms
rtt min/avg/max/mdev = 17.818/18.561/19.457/0.601 ms

lekto@blaszak ~ $ ping -4 github.com
PING github.com (140.82.121.3) 56(84) bytes of data.
64 bytes from lb-140-82-121-3-fra.github.com (140.82.121.3): icmp_seq=1 ttl=50 time=19.4 ms
64 bytes from lb-140-82-121-3-fra.github.com (140.82.121.3): icmp_seq=2 ttl=50 time=18.2 ms
64 bytes from lb-140-82-121-3-fra.github.com (140.82.121.3): icmp_seq=3 ttl=50 time=27.6 ms
64 bytes from lb-140-82-121-3-fra.github.com (140.82.121.3): icmp_seq=4 ttl=50 time=18.7 ms
^C
--- github.com ping statistics ---
4 packets transmitted, 4 received, 0% packet loss, time 3002ms
rtt min/avg/max/mdev = 18.240/20.992/27.623/3.852 ms

I did git bisect and it's showing me that this commit is causing this:

904cdb6dc9bd511847e31689873f9c57172f6b8d is the first bad commit
commit 904cdb6dc9bd511847e31689873f9c57172f6b8d
Author: Sami Kerola <kerolasa@iki.fi>
Date:   Sat Nov 30 19:14:39 2019 +0000

    ping: AF_INET6 is address family not socket type [lgtm scan]
    
    The lgtm scan gave rather strange warning 'Comparison is always false
    because socktype <= 3.' at this point in code.  Looking a little bit closer
    to issue I realized wrong variable was compared.  This seems to have slipped
    into ping five releases ago.
    
    Broken-since: d141cb6e4ee3388f0508afa0f6368aaa1236778c
    Signed-off-by: Sami Kerola <kerolasa@iki.fi>

 ping/ping.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

I'm using kernel without IPv6 support.

@pevik
Copy link
Contributor

pevik commented Sep 2, 2020

Thanks for your report. The commit is correct. But is there a regression? I don't thing so, but code could be improved.

  • For -4 ping creates only AF_INET socket.
  • For -6 ping creates onlyAF_INET6 socket.
  • For the default (AF_UNSPEC) ping creates both AF_INET andAF_INET6` socket.

iputils/ping/ping.c

Lines 523 to 533 in 4fd276c

if (hints.ai_family != AF_INET6)
create_socket(&rts, &sock4, AF_INET, hints.ai_socktype, IPPROTO_ICMP,
hints.ai_family == AF_INET);
if (hints.ai_family != AF_INET) {
create_socket(&rts, &sock6, AF_INET6, hints.ai_socktype, IPPROTO_ICMPV6, sock4.fd == -1);
/* This may not be needed if both protocol versions always had the same value, but
* since I don't know that, it's better to be safe than sorry. */
rts.pmtudisc = rts.pmtudisc == IP_PMTUDISC_DO ? IPV6_PMTUDISC_DO :
rts.pmtudisc == IP_PMTUDISC_DONT ? IPV6_PMTUDISC_DONT :
rts.pmtudisc == IP_PMTUDISC_WANT ? IPV6_PMTUDISC_WANT : rts.pmtudisc;
}

Then obviously correct socket is used. Ideally code would know which socket needs to use before trying to create it. I'll have to look more deeply into the code how big rewrite it will be (and I'd prefer to have some ping tests written before changing it unless the change is trivial). And at least the warning could print whether the error is on IPv4 or on IPv6 to be at least a less confusing.

@floppym
Copy link
Contributor

floppym commented Sep 2, 2020

I would guess that the desired outcome is to run ping github.com on a kernel with IPv6 disabled without seeing a warning message.

It is unlikely that a user who has disabled IPv6 support in the kernel would care that ping is unable to create an IPv6 socket.

@Whissi
Copy link
Contributor

Whissi commented Sep 2, 2020

...and keep in mind that github.com doesn't return any AAAA record at all. :)

@pevik pevik self-assigned this Sep 3, 2020
@klynastor
Copy link
Contributor

klynastor commented Nov 25, 2020

This broke nagios on me, since nagios expects no warnings sent to stderr. Here's a quick patch.

--- iputils-s20200821/ping/ping.c.bak   2020-11-25 00:22:54.000000000 -0500
+++ iputils-s20200821/ping/ping.c       2020-11-25 00:23:13.000000000 -0500
@@ -150,8 +150,8 @@
                /* Report error related to disabled IPv6 only when IPv6 also failed or in
                 * verbose mode. Report other errors always.
                 */
-               if ((errno == EAFNOSUPPORT && family == AF_INET6) ||
-                   rts->opt_verbose || requisite)
+               if ((errno == EAFNOSUPPORT && family == AF_INET6 && requisite) ||
+                   rts->opt_verbose)
                        error(0, errno, "socket");
                if (requisite)
                        exit(2);

@clayne
Copy link

clayne commented Feb 4, 2021

This is absolutely a regression. If a user pings a destination on an IPv4 only host, they should not suddenly be seeing a new warning just because socket(AF_INET6, SOCK_DGRAM, IPPROTO_ICMPV6) failed.

clayne@dorian:~ $ sudo strace ping -c1 192.168.1.1
[...]
socket(AF_INET, SOCK_DGRAM, IPPROTO_ICMP) = -1 EACCES (Permission denied)
socket(AF_INET, SOCK_RAW, IPPROTO_ICMP) = 3
socket(AF_INET6, SOCK_DGRAM, IPPROTO_ICMPV6) = -1 EAFNOSUPPORT (Address family not supported by protocol)
write(2, "ping: ", 6ping: )                   = 6
write(2, "socket", 6socket)                   = 6
write(2, ": Address family not supported b"..., 42: Address family not supported by protocol) = 42
write(2, "\n", 1
)                       = 1

This is the commit that first introduced this behavior based on a bisect:

904cdb6dc9bd511847e31689873f9c57172f6b8d is the first bad commit
commit 904cdb6dc9bd511847e31689873f9c57172f6b8d
Author: Sami Kerola <kerolasa@iki.fi>
Date:   Sat Nov 30 19:14:39 2019 +0000

    ping: AF_INET6 is address family not socket type [lgtm scan]
    
    The lgtm scan gave rather strange warning 'Comparison is always false
    because socktype <= 3.' at this point in code.  Looking a little bit closer
    to issue I realized wrong variable was compared.  This seems to have slipped
    into ping five releases ago.
    
    Broken-since: d141cb6e4ee3388f0508afa0f6368aaa1236778c
    Signed-off-by: Sami Kerola <kerolasa@iki.fi>

 ping/ping.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
bisect run success

clayne@dorian:/tmp/iputils/iputils ((904cdb6...)|BISECTING) $ git diff -u 904cdb6dc9bd511847e31689873f9c57172f6b8d~1
diff --git a/ping/ping.c b/ping/ping.c
index 8d15422..bcb88a8 100644
--- a/ping/ping.c
+++ b/ping/ping.c
@@ -149,7 +149,7 @@ static void create_socket(struct ping_rts *rts, socket_st *sock, int family,
                /* Report error related to disabled IPv6 only when IPv6 also failed or in
                 * verbose mode. Report other errors always.
                 */
-               if ((errno == EAFNOSUPPORT && socktype == AF_INET6) ||
+               if ((errno == EAFNOSUPPORT && family == AF_INET6) ||
                    rts->opt_verbose || requisite)
                        error(0, errno, "socket");
                if (requisite)

Presumably what happened here is that once the right variable was used this warning started being emitted in cases where AF_INET6 is not supported and nobody caught this because either the committers or build infrastructure is entirely dual-stack? However, it seems like the change above is merely a red herring and merely made the check start working correctly. IMO, the real issue started back in 9fd870a once the conditional itself was changed.

cc @pavlix @kerolasa

@pevik pevik pinned this issue Feb 4, 2021
@pevik pevik added the bug label Feb 4, 2021
@pevik
Copy link
Contributor

pevik commented Feb 4, 2021

@lekto @clayne Could you please share your environment? I'm able to reproduce the problem with ping -6 github.com on kernel without IPv6 support (which is correct behavior), but with just ping github.com I get both sock4.fd and sock6.fd as valid sockets.

sysctl -w net.ipv6.conf.all.disable_ipv6=1; sysctl -w net.ipv6.conf.default.disable_ipv6=1 # disable IPv6
./builddir/ping/ping -c1 -v github.com
PING github.com (140.82.121.3) 56(84) bytes of data.
64 bytes from lb-140-82-121-3-fra.github.com (140.82.121.3): icmp_seq=1 ttl=48 time=16.6 ms

@lekto
Copy link
Author

lekto commented Feb 6, 2021

Both my PC and my router: Gentoo ~amd64 (unstable), kernel: gentoo-sources-5.10.12 configured from scratch by me and iputils-20210202. In attachment there is a build log from iputils.

build.log

@Piraty
Copy link

Piraty commented Feb 15, 2021

$ cat /proc/cmdline
<...> ipv6.disable=1

$ uname -m -r
5.10.15_1 x86_64

$ iputils-ping -V
ping from iputils s20200821

$ iputils-ping -c1 9.9.9.9 ; echo "RET: $?"
iputils-ping: socket: Address family not supported by protocol
PING 9.9.9.9 (9.9.9.9) 56(84) bytes of data.
64 bytes from 9.9.9.9: icmp_seq=1 ttl=55 time=13.7 ms
<...>
RET: 0

@pevik
Copy link
Contributor

pevik commented Feb 15, 2021

Thanks a lot for a reproducer. I tried to disable with sysctl -w net.ipv6.conf.all.disable_ipv6=1; sysctl -w net.ipv6.conf.default.disable_ipv6=1 which is not obviously enough (full disable on kernel cmdline is needed).
I'm busy with other tasks but try to have look into this soon.

@imThief
Copy link

imThief commented Mar 6, 2021

Hello!
I am experiencing same problem, ipv6 is not disabled - its not supported (CONFIG_IPV6 is not set) and commit with "if ((errno == EAFNOSUPPORT && family == AF_INET6)" change is already in, but error message still displays... Can you help?

my kernel is 5.4.97-gentoo-x86_64
[I] net-misc/iputils 20200821-r2(04:16:50 PM 02/23/2021)(arping filecaps nls ssl -caps -clockdiff -doc -gcrypt -idn -ipv6 -libressl -nettle -rarpd -rdisc -static -tftpd -tracepath -traceroute6)

Thanks!

UPD: recompiling kernel with CONFIG_IPV6=y removes that warn message, i suppose that kernel return something other than EAFNOSUPPORT when CONFIG_IPV6 is not set...

@Vaevictusnet
Copy link

So, to avoid the warning, any system with ipv6 disabled (for security reasons, for example) will have patch and recompile all systems that previously used ping for monitoring. my nagios blew up.

@Vaevictusnet
Copy link

looks like the nagios piece does ignore the -4 flag as well.

@pevik
Copy link
Contributor

pevik commented Apr 27, 2021

I'm sorry, I have it on my TODO list, but have no idea when I get time for it.

@Vaevictusnet
Copy link

Vaevictusnet commented Apr 28, 2021 via email

@pevik
Copy link
Contributor

pevik commented Apr 28, 2021

... There may be something that can be done on the nagios side, to force the -4 flag, but i'm unsure how that would work with backwards compatibility and some other things.

According to parameters in check_ping doc is implemented in C: check_ping.c.
Because there are also other third party implementations: perl and shell.

The official C implementation detects supported command, in my case config.h has

/* path and args for ICMPv6 ping command */
#define PING6_COMMAND "/usr/bin/ping6 -n -U -w %d -c %d %s"

/* Define if packet count must precede host */
#define PING6_PACKETS_FIRST 1

/* path and args for ICMP ping command */
#define PING_COMMAND "/usr/bin/ping -n -U -w %d -c %d %s"

/* Define if ping has its own timeout option that should be set */
#define PING_HAS_TIMEOUT 1

/* Define if packet count must precede host */
#define PING_PACKETS_FIRST 1

=> no -4 or -6 defined. And I suppose Nagios also supports FreeBSD ping, which does not have these switches.
If you're patient enough, we'll fix this issue.

@redtux
Copy link

redtux commented Jun 8, 2021

I am having the same issue as @lekto with iputils-ping 3:20210202-1 on Ubuntu 21.04 hirsute (rpi4/aarch64) and IPv6 disabled (via kernel parameter ipv6.disable=1).

ping -V
ping from iputils 20210202

ip a s eth0
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP group default qlen 1000
    link/ether dc:a6:32:xx:xx:xx brd ff:ff:ff:ff:ff:ff
    inet 192.168.xxx.xxx/24 brd 192.168.xxx.255 scope global dynamic eth0
       valid_lft 77396sec preferred_lft 77396sec

uname -r
5.11.0-1009-raspi

@Ulenrich
Copy link

The command has no effect if ipv6 is disabled in the kernel:

sysctl -w net.ipv6.conf.all.disable_ipv6=1
sysctl: cannot stat /proc/sys/net/ipv6/conf/all/disable_ipv6: No such file or directory

@ruyrybeyro
Copy link

ruyrybeyro commented Aug 14, 2021

Having issues in Debian 11/amd64, as @redtux , IPv6 disabled via kernel parameter:

# ping 6.6.6.6
ping: socket: Address family not supported by protocol
PING 6.6.6.6 (6.6.6.6) 56(84) bytes of data.

# ping -V
ping from iputils 20210202 

# dpkg -l iputils-ping | grep ^i
ii  iputils-ping   3:20210202-1 amd64        Tools to test the reachability of network hosts

# ip a s eth0
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP group default qlen 1000
    link/ether 00:50:56:36:4d:90 brd ff:ff:ff:ff:ff:ff
    inet 192.168.5.2/24 brd 192.168.5.255 scope global eth0
       valid_lft forever preferred_lft forever

# cat /proc/cmdline 
BOOT_IMAGE=/boot/vmlinuz-5.10.0-3-amd64 root=/dev/sda1 ro noapic elevator=noop audit=0 ipv6.disable=1 consoleblank=0 quiet

@takenek
Copy link

takenek commented Sep 11, 2021

Same here on debian 11.

root@smokeping:~# cat /proc/cmdline
BOOT_IMAGE=/vmlinuz-5.10.0-8-amd64 root=/dev/mapper/smokeping--vg-root ro ipv6.disable=1 quiet

root@smokeping:~# cat /etc/gai.conf
precedence ::ffff:0:0/96 100

root@smokeping:~# ping 8.8.8.8 -c 1
ping: socket: Address family not supported by protocol
PING 8.8.8.8 (8.8.8.8) 56(84) bytes of data.
64 bytes from 8.8.8.8: icmp_seq=1 ttl=113 time=1.2 ms

root@smokeping:~# dpkg -l |grep iputils-ping
ii iputils-ping 3:20210202-1 amd64 Tools to test the reachability of network hosts

Besr Regards

@ac000
Copy link

ac000 commented Sep 16, 2021

Just had a look at this (needed to boot with ipv6.disable=1 to reproduce the issue) and I arrived at the same patch by @klynastor

--- iputils-s20200821/ping/ping.c.bak   2020-11-25 00:22:54.000000000 -0500
+++ iputils-s20200821/ping/ping.c       2020-11-25 00:23:13.000000000 -0500
@@ -150,8 +150,8 @@
                /* Report error related to disabled IPv6 only when IPv6 also failed or in
                 * verbose mode. Report other errors always.
                 */
-               if ((errno == EAFNOSUPPORT && family == AF_INET6) ||
-                   rts->opt_verbose || requisite)
+               if ((errno == EAFNOSUPPORT && family == AF_INET6 && requisite) ||
+                   rts->opt_verbose)
                        error(0, errno, "socket");
                if (requisite)
                        exit(2);

You need to make requisite part of the first half of the logic, so that other than if verbose is set we only print the socket error if we failed on IPv6 and failed on IPv4. Or we are only trying and failed on IPv6.

pevik added a commit to pevik/iputils that referenced this issue Sep 16, 2021
Regression was introduced in d141cb6 as introduced condition

if ((errno == EAFNOSUPPORT && socktype == AF_INET6) || options & F_VERBOSE || requisite)

was wrong, it should have been:

if ((errno == EAFNOSUPPORT && family == AF_INET6 && requisite) || options & F_VERBOSE)

but bug was hidden as `family == AF_INET6' was always false until
otherwise correct fix 904cdb6 ("ping: AF_INET6 is address family not
socket type [lgtm scan]") propagated the error.

Fixes: d141cb6 ("ping: work with older kernels that don't support ping sockets")
Closes: iputils#293

Reported-by: lekto <lekto@o2.pl>
Suggested-by: klynastor
Signed-off-by: Petr Vorel <pvorel@suse.cz>
@pevik
Copy link
Contributor

pevik commented Sep 16, 2021

Fix is ready, anybody feel free to test and put your Reviewed-by: Your Name <your@email> tag.
I'm sorry it took me one year to fix this simple issue.

pevik added a commit to pevik/iputils that referenced this issue Sep 16, 2021
Regression was introduced in d141cb6 as introduced condition

if ((errno == EAFNOSUPPORT && socktype == AF_INET6) || options & F_VERBOSE || requisite)

was wrong, it should have been:

if ((errno == EAFNOSUPPORT && family == AF_INET6 && requisite) || options & F_VERBOSE)

but bug was hidden as `family == AF_INET6' was always false until
otherwise correct fix 904cdb6 ("ping: AF_INET6 is address family not
socket type [lgtm scan]") propagated the error.

Tested on kernel booted with ipv6.disable=1 (disabling via sysctl, i.e.
sysctl -w net.ipv6.conf.all.disable_ipv6=1; sysctl -w net.ipv6.conf.default.disable_ipv6=1
does not trigger the issue as it exit with "socket: Address family not
supported by protocol" - errno EADDRNOTAVAIL).

Fixes: d141cb6 ("ping: work with older kernels that don't support ping sockets")
Closes: iputils#293

Reported-by: lekto <lekto@o2.pl>
Suggested-by: klynastor
Signed-off-by: Petr Vorel <pvorel@suse.cz>
pevik added a commit to pevik/iputils that referenced this issue Sep 16, 2021
Regression was introduced in d141cb6 as introduced condition

if ((errno == EAFNOSUPPORT && socktype == AF_INET6) || options & F_VERBOSE || requisite)

was wrong, it should have been:

if ((errno == EAFNOSUPPORT && family == AF_INET6 && requisite) || options & F_VERBOSE)

but bug was hidden as `family == AF_INET6' was always false until
otherwise correct fix 904cdb6 ("ping: AF_INET6 is address family not
socket type [lgtm scan]") propagated the error.

Tested on kernel booted with ipv6.disable=1 (disabling via sysctl, i.e.
sysctl -w net.ipv6.conf.all.disable_ipv6=1; sysctl -w net.ipv6.conf.default.disable_ipv6=1
does not trigger the issue as it exit with "socket: Address family not
supported by protocol" - errno EADDRNOTAVAIL).

Fixes: d141cb6 ("ping: work with older kernels that don't support ping sockets")
Closes: iputils#293

Reported-by: lekto <lekto@o2.pl>
Suggested-by: klynastor
Reviewed-by: Andrew Clayton <andrew@digital-domain.net>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
@imThief
Copy link

imThief commented Sep 16, 2021

I'm sorry, is this commit already in master branch? Can i just clone it, compile and test or should i choose some side branch? Work with git unfortunately not among my virtues ...

@ac000
Copy link

ac000 commented Sep 17, 2021

@imThief

If you already have this repository checked out, you can simply pull this change over with

git pull https://github.com/pevik/iputils/ ping/gh#293

pevik pushed a commit to pevik/iputils that referenced this issue Sep 17, 2021
Regression was introduced in d141cb6 as introduced condition

if ((errno == EAFNOSUPPORT && socktype == AF_INET6) || options & F_VERBOSE || requisite)

was wrong, it should have been:

if ((errno == EAFNOSUPPORT && family == AF_INET6 && requisite) || options & F_VERBOSE)

but bug was hidden as `family == AF_INET6' was always false until
otherwise correct fix 904cdb6 ("ping: AF_INET6 is address family not
socket type [lgtm scan]") propagated the error.

Tested on kernel booted with ipv6.disable=1 (disabling via sysctl, i.e.
sysctl -w net.ipv6.conf.all.disable_ipv6=1; sysctl -w net.ipv6.conf.default.disable_ipv6=1
does not trigger the issue as it exit with "socket: Address family not
supported by protocol" - errno EADDRNOTAVAIL).

Fixes: d141cb6 ("ping: work with older kernels that don't support ping sockets")
Closes: iputils#293

Reported-by: lekto <lekto@o2.pl>
Reviewed-by: Andrew Clayton <andrew@digital-domain.net>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Byron Stanoszek <gandalf@winds.org>
[ pvorel: create commit from Byron's patch on the issue, do analysis and wrote commit message ]
Signed-off-by: Petr Vorel <pvorel@suse.cz>
@pevik pevik closed this as completed in 79d713e Sep 19, 2021
@pevik
Copy link
Contributor

pevik commented Sep 19, 2021

@imThief FYI fix merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.