Hi Pedro, No, this is not right. Let me clarify:
(1) I never told that there are any issues with the tests. I just mumbled that the tests should catch such regression though I never checked if they actually did. (2) The MAXPAYLOAD calculation in the code below is not fully correct. It should be 65535 - 20 (ip header) - 8 (icmp part) = 65507 without IP options OR 65535 - 20 (ip header) - 40 (ip options) - 8 (icmp part) = 65467 with IP options, ie. whenever you run ping -R. The code below hardcoded the latter value which is simply wrong. I wouldn't rely on the fact that you get it from other BSD flavours and would recommend to have this code reviewed before committing it. Maxim On Wed, 16 Oct 2024, 18:40-0000, Pedro F. Giffuni wrote: > The branch main has been updated by pfg: > > URL: > https://cgit.FreeBSD.org/src/commit/?id=b88df1e893c455731c7915f72a3b97b078ab04e2 > > commit b88df1e893c455731c7915f72a3b97b078ab04e2 > Author: Pedro F. Giffuni <p...@freebsd.org> > AuthorDate: 2024-10-16 18:35:44 +0000 > Commit: Pedro F. Giffuni <p...@freebsd.org> > CommitDate: 2024-10-16 18:39:48 +0000 > > Reapply "sbin/ping: allow normal users to specify larger packets" > > The ping tests were originally broken by an unrelated isue that > is now fixed ( 2926c2594249f64fecbbdcb0b0b9a3591931ab04 ). > > THanks to kp@ for fixing the test and Jose Luis Duran for pinting it out. > > This reverts commit 7bc0cb91a2dfc7e23d96efd0fb7866ee2a23ba88. > --- > sbin/ping/ping.8 | 3 +-- > sbin/ping/ping.c | 11 +++++------ > 2 files changed, 6 insertions(+), 8 deletions(-) > > diff --git a/sbin/ping/ping.8 b/sbin/ping/ping.8 > index 0eaec196e1e3..951049d0f252 100644 > --- a/sbin/ping/ping.8 > +++ b/sbin/ping/ping.8 > @@ -25,7 +25,7 @@ > .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF > .\" SUCH DAMAGE. > .\" > -.Dd September 15, 2023 > +.Dd October 15, 2024 > .Dt PING 8 > .Os > .Sh NAME > @@ -312,7 +312,6 @@ with the 8 bytes of > ICMP > header data. > .Pp > -For IPv4, only the super-user may specify values more than default. > This option cannot be used with ping sweeps. > .Pp > For IPv6, you may need to specify > diff --git a/sbin/ping/ping.c b/sbin/ping/ping.c > index d9d544bc75c8..e6b1247af497 100644 > --- a/sbin/ping/ping.c > +++ b/sbin/ping/ping.c > @@ -96,8 +96,8 @@ > #define DEFDATALEN 56 /* default data length */ > #define FLOOD_BACKOFF 20000 /* usecs to back off if F_FLOOD > mode */ > /* runs out of buffer space */ > -#define MAXIPLEN (sizeof(struct ip) + MAX_IPOPTLEN) > -#define MAXICMPLEN (ICMP_ADVLENMIN + MAX_IPOPTLEN) > +#define MAXIPLEN ((int)sizeof(struct ip) + MAX_IPOPTLEN) > +#define MAXPAYLOAD (IP_MAXPACKET - MAXIPLEN - ICMP_MINLEN) > #define MAXWAIT 10000 /* max ms to wait for response > */ > #define MAXALARM (60 * 60) /* max seconds for alarm > timeout */ > #define MAXTOS 255 > @@ -458,11 +458,10 @@ ping(int argc, char *const *argv) > errx(EX_USAGE, "invalid packet size: `%s'", > optarg); > datalen = (int)ltmp; > - if (uid != 0 && datalen > DEFDATALEN) { > - errno = EPERM; > - err(EX_NOPERM, > + if (datalen > MAXPAYLOAD) { > + errx(EX_USAGE, > "packet size too large: %d > %u", > - datalen, DEFDATALEN); > + datalen, MAXPAYLOAD); > } > break; > case 'T': /* multicast TTL */ > -- Maxim Konovalov
