Ping: checks payloads incorrectly, ignores all mismatch replies

The issue is fixed in the version available in the current Ubuntu serie

and I've uploaded a focal SRU candidate to the review queue now

I checked that jammy and later indeed has the same commit reverted.

This is definitely a bug that we want to fix in Focal. But is this particular code change the correct way to do it?

Won't reverting this commit regress the fix that was introduced by the commit in the first place? I think this needs a proper analysis of the possible implications for unaffected users, please, rather than just blindly cherry-picking the revert from upstream.

I think maybe bug 1551020 would be reintroduced by this upload, for example?

The test plan should also include verifying regular use cases that might be impacted by this code change are not regressed, not just that this particular bug is fixed.

@Robie, the patch description has details on the change. The github report also discuss why the patch was broken and not just doing what is intended and why the revert is right.

I'm not involved in iputils and will not be able to provide more information that was in there. Note that the revert is part of 22.04.

I was mostly trying to help landing a fix we were getting request on. I'm fine updating the testcase if that's enough to unblock the SRU but otherwise please reject my upload, I'm not wanting to spend the time needed to prove that upstream can be trusted on that one.

I'm not doubting that the revert was correct for upstream. But we have a different context in a stable release, where users may be relying on behaviours that upstream are at liberty to change in their main development branch, but we are not in our stable releases. Further, we can't just assume that what is correct for them in their code base is correct for us to cherry-pick to our stable release that holds a previous version of the code base. Somebody needs to spend time to ensure this is safe.

The description of the patch explains why the current behavior is buggy and not just different in a way users would be relying on the difference of behavior. It's not that it works differently, it is that it reports wrong information.

The patch didn't apply directly so I've adapted it and verified that a build of the package with it matched the new expected behavior before uploading.

I agree with Seb. Behavior is broken, not different. It's not just about printing information; it's the fact that ping is not checking csum, not checking duplicates... Ideally, both this and bug 1551020 should be fixed. But if I'd have to pick only one, this would be the one.

Hello Matt, or anyone else affected,

Accepted iputils into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/iputils/3:20190709-3ubuntu1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

All autopkgtests for the newly accepted iputils (3:20190709-3ubuntu1) for focal have finished running.
The following regressions have been reported in tests triggered by the package:

systemd/245.4-4ubuntu3.18 (armhf)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/focal/update_excuses.html#iputils

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

Re-triggered the failing autopkgtest on armhf.

The armhf test seems to have been broken for a bit, in root-unittests:

test_exec_dynamicuser: exec-dynamicuser-statedir.service: exit status 1, expected 0
FAIL: test-execute (code: 134)

The DEP8 tests are green now, but we still need verification on this bug.

The fix for this bug has been awaiting testing feedback in the -proposed repository for focal for more than 90 days. Please test this fix and update the bug appropriately with the results. In the event that the fix for this bug is still not verified 15 days from now, the package will be removed from the -proposed repository.

Focal verification

# Reproducing the bug

ubuntu@f-ping-1964506:~$ apt-cache policy iputils-ping
iputils-ping:
  Installed: 3:20190709-3
  Candidate: 3:20190709-3
  Version table:
 *** 3:20190709-3 500
        500 http://br.archive.ubuntu.com/ubuntu focal/main amd64 Packages
        100 /var/lib/dpkg/status

ubuntu@f-ping-1964506:~$ ping -c 1 -s 1200 8.8.8.8
PING 8.8.8.8 (8.8.8.8) 1200(1228) bytes of data.

--- 8.8.8.8 ping statistics ---
1 packets transmitted, 0 received, 100% packet loss, time 0ms

# Confirming the fix with the package from proposed

ubuntu@f-ping-1964506:~$ apt-cache policy iputils-ping
iputils-ping:
  Installed: 3:20190709-3ubuntu1
  Candidate: 3:20190709-3ubuntu1
  Version table:
 *** 3:20190709-3ubuntu1 500
        500 http://br.archive.ubuntu.com/ubuntu focal-proposed/main amd64 Packages
        100 /var/lib/dpkg/status
     3:20190709-3 500
        500 http://br.archive.ubuntu.com/ubuntu focal/main amd64 Packages

ubuntu@f-ping-1964506:~$ ping -c 1 -s 1200 8.8.8.8
PING 8.8.8.8 (8.8.8.8) 1200(1228) bytes of data.
76 bytes from 8.8.8.8: icmp_seq=1 ttl=118 (truncated)

--- 8.8.8.8 ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 9.690/9.690/9.690/0.000 ms
ubuntu@f-ping-1964506:~$

# Focal verification succeeded

The verification of the Stable Release Update for iputils has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

This bug was fixed in the package iputils - 3:20190709-3ubuntu1

---------------
iputils (3:20190709-3ubuntu1) focal; urgency=medium

  * debian/patches/git_revert_strict_pattern_matching.patch:
    - cherrypick of an upstream revert of a buggy commit which was leading
      to incorrect ping statistics on truncated packets (lp: #1964506)

 -- Sebastien Bacher <email address hidden> Tue, 13 Sep 2022 21:06:15 +0200