Sorry for slow reply... On Mon, Jan 25, 2010 at 05:31:39AM -0500, Thomas Guyot-Sionnest wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On 22/01/10 06:41 PM, Jan Wagner wrote: > > Hi Matija, > > > > I just fixed #534604 but not #51458, the changelog is just wrong. But I > > pinged > > upstream about you patch, if its a valid change for all conditions ... > > Actually there is just more evaluation needed by upstream. > > I have looked a bit into this, I'm just afraid of changing it without > the ability to test myself as it has the potential to break many other > architecture I don't have access to. > > If you can give me more details like which OS, software version and > network condition (i.e. what happen when you get "+n errors"). Id your > OS is Linux x86 32 or 64 bit you could even send me the binary and I may > be able to run it locally to reproduce it.
It was Debian Etch when it was reported, now it is debian Lenny (same symptoms, patch is just a few lines lower but applies cleanly). Package with ping6 (in Lenny) used is iputils-ping 3:20071127-1 so you can grab the binaries from http://packages.debian.org/search?keywords=iputils-ping It is the same bug with either i386 or amd64 architecture. To reproduce, here is example I get when I try to ping nonexistent IPv6 address (your output might be different due to routers involved, but hopefully would be in the same format): % ping6 -c3 2a00:1450:8001::1234 PING 2a00:1450:8001::1234(2a00:1450:8001::1234) 56 data bytes >From 2001:4860:0:1::1d icmp_seq=2 Destination unreachable: Address unreachable >From 2001:4860:0:1::1b icmp_seq=3 Destination unreachable: Address unreachable --- 2a00:1450:8001::1234 ping statistics --- 3 packets transmitted, 0 received, +2 errors, 100% packet loss, time 2003ms when you try to use check_ping v1991 (nagios-plugins 1.4.12) you get % ./check_ping -H 2a00:1450:8001::1234 -w 5000,100% -c 5000,100% -p 1 /bin/ping6 -n -U -w 10 -c 1 2a00:1450:8001::1234 CRITICAL - Could not interpret output from ping command Now, if you apply the patch ping-sscanf.diff from http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=514588 you will get the correct answer: % ./check_ping -H 2a00:1450:8001::1234 -w 5000,100% -c 5000,100% -p 1 PING CRITICAL - Packet loss = 100%|rta=5000.000000ms;5000.000000;5000.000000;0.000000 pl=100%;100;100;0 (note that in our real world case we actually had a flaky network and the IPv6 address was right, but the output format was the same, only we did not have 100% lost but less etc -- but we couldn't get that data from check_ping plugin as it couldn't interpret the output). The problematic code lies in check_ping.c around line 455: 455 if(sscanf(buf,"%*d packets transmitted, %*d packets received, +%*d errors, %d%% packet loss",&pl)==1 || 456 sscanf(buf,"%*d packets transmitted, %*d packets received, +%*d duplicates, %d%% packet loss", &pl) == 1 || 457 sscanf(buf,"%*d packets transmitted, %*d received, +%*d duplicates, %d%% packet loss", &pl) == 1 || 458 sscanf(buf,"%*d packets transmitted, %*d packets received, %d%% packet loss",&pl)==1 || 459 sscanf(buf,"%*d packets transmitted, %*d packets received, %d%% loss, time",&pl)==1 || 460 sscanf(buf,"%*d packets transmitted, %*d received, %d%% loss, time", &pl)==1 || 461 sscanf(buf,"%*d packets transmitted, %*d received, %d%% packet loss, time", &pl)==1 || 462 sscanf(buf,"%*d packets transmitted, %*d received, +%*d errors, %d%% packet loss", &pl) == 1 || 463 sscanf(buf,"%*d packets transmitted %*d received, +%*d errors, %d%% packet loss", &pl) == 1 As you can see, our summary line from ping6 "3 packets transmitted, 0 received, +2 errors, 100% packet loss, time 2003ms" should have matched at line 462, at: sscanf(buf,"%*d packets transmitted, %*d received, +%*d errors, %d%% packet loss", &pl) == 1 as that is the only line with string that would match, right ? unfortunately, it will actually match at line 460, at sscanf(buf,"%*d packets transmitted, %*d received, %d%% loss, time", &pl)==1 which is completely wrong, as that is not our string at all! Careful reading of sscanf(3) reveals the problem -- the sscanf(3) does not at all work like perl or similar languages. The return value of sscanf(3) is instead number of arguments matched, and *not* the claim that the *whole* string matched. That is, any and all text after last "%d" is completely ignored (that is, no matter if they match or not, the sscanf will return the same value - number of % arguments matched so far: 1 in our case). So line 460 might as well be written as: sscanf(buf,"%*d packets transmitted, %*d received, %d", &pl)==1 which shows more clearly why it breaks. Now the patch might look like it changes a lot but that is only because diff is line oriented -- we're actually just adding "%n" at the end of each line (and &match variable to store it) to be sure *the whole* line matched, not just a part up to last "%d". I hope this gives you enough info to check and validate the patch. Thanks, Matija Nalis -- To UNSUBSCRIBE, email to [email protected] with a subject of "unsubscribe". Trouble? Contact [email protected]
