From nobody Wed Oct 16 19:04:53 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4XTL4m20CLz5YtF6; Wed, 16 Oct 2024 19:04:56 +0000 (UTC) (envelope-from maxim@maxim.int.ru) Received: from maxim.int.ru (maxim.int.ru [167.71.75.67]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "maxim.int.ru", Issuer "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4XTL4l3RXYz41Yk; Wed, 16 Oct 2024 19:04:55 +0000 (UTC) (envelope-from maxim@maxim.int.ru) Authentication-Results: mx1.freebsd.org; none Received: from localhost (maxim@localhost [127.0.0.1]) by maxim.int.ru (8.17.1/8.17.1) with ESMTPS id 49GJ4rSl029948 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NO); Wed, 16 Oct 2024 19:04:53 GMT (envelope-from maxim@maxim.int.ru) Date: Wed, 16 Oct 2024 19:04:53 +0000 (UTC) From: Maxim Konovalov To: "Pedro F. Giffuni" cc: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: Re: git: b88df1e893c4 - main - Reapply "sbin/ping: allow normal users to specify larger packets" In-Reply-To: <202410161840.49GIe8CR000407@gitrepo.freebsd.org> Message-ID: References: <202410161840.49GIe8CR000407@gitrepo.freebsd.org> List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:14061, ipnet:167.71.64.0/20, country:US] X-Rspamd-Queue-Id: 4XTL4l3RXYz41Yk X-Spamd-Bar: ---- 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 > AuthorDate: 2024-10-16 18:35:44 +0000 > Commit: Pedro F. Giffuni > 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