sbin/ping: allow normal users to specify larger packets

[pfg]

Summary

Only super-user could specify a packet size larger than the default 56 bytes.
This restriction was added by Matt Dillon in 1998 during the BEST days [0].
This restriction doesn't exist in ping IPV6 or on NetBSD, OpenBSD and Linux.

UMS [1] uses this feature to estimate the client's bandwidth to optimize the
streaming experience.

[0] Git 526f06b278d9252add168aa18b60242c08771165
[1] UMS: https://github.com/UniversalMediaServer/UniversalMediaServer

Obtained from: DragonFlyBSD

Sidenotes: MAXICMPLEN seems to be unused in the current code and I had to sign cast a value in MAXIPLEN to make the compiler happy.

[zlei]

So the kernel does not have a real limit ( only super-user could specify a packet size larger than the default 56 bytes ) ?

[delphij]

sbin/ping/ping.c

461–464

I think this is actually redundant (code below would check and calculate a more accurate limit).

[pfg]

sbin/ping/ping.c

461–464

While the check does seem redundant and is only for non-root, the case only applies when "-s" is specified. Other checks may not apply

Hmm... would you mean just making -s a no-op?

[jlduran]

@kp has fixed the ping test in 2926c2594249f64fecbbdcb0b0b9a3591931ab04. It was not related to this commit.

[maxim]

Hi Pedro,

I would recommend to take a look at Gleb's patch there:

https://lists.freebsd.org/archives/dev-commits-src-all/2024-October/047042.txt

I also had this approach in mind when I saw your change.

Gleb's version has several advantages:

• it addresses all cases where the payload is bigger than DEFDATALEN, i.e., -s and sweep options;

• it fixes setsockopt(2) call for non-root

• it doesn't break maximum payload size calculation for different use cases

It also makes sense to extend the tests to cover this change (I might want to look at this part myself).

Some general notes.

Please note also that our ping(8) diverged from other BSD's greatly. So it is often simply impossible to backport patches from there without significant refactoring.

Also, Gleb has the concerns about the security implications with this change but I'd suggest to discuss this topic with him separately.

I know nothing about UMS and don't really have any resources to learn about it more at the moment so I might be totally off the track here but based on my experience the idea to check end-to-end bandwidth with icmp, while might work in some cases, is quite unreliable in general.