From owner-svn-src-head@freebsd.org Tue Feb 25 13:03:12 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 6B55B25B5A1; Tue, 25 Feb 2020 13:03:12 +0000 (UTC) (envelope-from bz@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 48RfHh2J3qz3KJl; Tue, 25 Feb 2020 13:03:12 +0000 (UTC) (envelope-from bz@FreeBSD.org) Received: from mx1.sbone.de (mx1.sbone.de [IPv6:2a01:4f8:13b:39f::9f:25]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mx1.sbone.de", Issuer "SBone.DE" (not verified)) (Authenticated sender: bz/mail) by smtp.freebsd.org (Postfix) with ESMTPSA id 1D1332E82F; Tue, 25 Feb 2020 13:03:12 +0000 (UTC) (envelope-from bz@FreeBSD.org) Received: from mail.sbone.de (mail.sbone.de [IPv6:fde9:577b:c1a9:31::2013:587]) (using TLSv1 with cipher ADH-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by mx1.sbone.de (Postfix) with ESMTPS id E106A8D4A516; Tue, 25 Feb 2020 13:03:09 +0000 (UTC) Received: from content-filter.sbone.de (content-filter.sbone.de [IPv6:fde9:577b:c1a9:31::2013:2742]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.sbone.de (Postfix) with ESMTPS id CA160E707C4; Tue, 25 Feb 2020 13:03:09 +0000 (UTC) X-Virus-Scanned: amavisd-new at sbone.de Received: from mail.sbone.de ([IPv6:fde9:577b:c1a9:31::2013:587]) by content-filter.sbone.de (content-filter.sbone.de [fde9:577b:c1a9:31::2013:2742]) (amavisd-new, port 10024) with ESMTP id UAN3Y7N8seYe; Tue, 25 Feb 2020 13:03:08 +0000 (UTC) Received: from [169.254.231.217] (unknown [IPv6:fde9:577b:c1a9:4902:998:d647:456f:d093]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.sbone.de (Postfix) with ESMTPSA id DC540E707B5; Tue, 25 Feb 2020 13:03:06 +0000 (UTC) From: "Bjoern A. Zeeb" To: "Kristof Provost" Cc: "Hans Petter Selasky" , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r358167 - head/sys/netinet6 Date: Tue, 25 Feb 2020 13:03:05 +0000 X-Mailer: MailMate (2.0BETAr6146) Message-ID: In-Reply-To: <963E6D27-9320-4F84-A3BB-A597C829AAAB@FreeBSD.org> References: <202002201056.01KAuC0N029186@repo.freebsd.org> <7d9ea227-7208-5773-080c-5ebc365c4be0@selasky.org> <963E6D27-9320-4F84-A3BB-A597C829AAAB@FreeBSD.org> MIME-Version: 1.0 Content-Type: text/plain; format=flowed X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 25 Feb 2020 13:03:12 -0000 On 25 Feb 2020, at 12:44, Kristof Provost wrote: >> This change introduces a slight regression when a host replies to >> IPv6 ping fragmented packets. The problem is the "unfragpartlen" must >> also be set in the else case of "if (opt)", else the payload offset >> computation for IPv6 fragments goes wrong by the size of the IPv6 >> header! >> > Confirmed, because the pf fragmentation:v6 test also fails on this. >> Patch goes like this: >> >>> diff --git a/sys/netinet6/ip6_output.c b/sys/netinet6/ip6_output.c >>> index 06c57bcec48..a6c8d148833 100644 >>> --- a/sys/netinet6/ip6_output.c >>> +++ b/sys/netinet6/ip6_output.c >>> @@ -459,7 +459,6 @@ ip6_output(struct mbuf *m0, struct ip6_pktopts >>> *opt, >>> */ >>> bzero(&exthdrs, sizeof(exthdrs)); >>> optlen = 0; >>> - unfragpartlen = 0; >>> if (opt) { >>> /* Hop-by-Hop options header. */ >>> MAKE_EXTHDR(opt->ip6po_hbh, &exthdrs.ip6e_hbh, >>> optlen); >>> @@ -497,8 +496,6 @@ ip6_output(struct mbuf *m0, struct ip6_pktopts >>> *opt, >>> /* Routing header. */ >>> MAKE_EXTHDR(opt->ip6po_rthdr, &exthdrs.ip6e_rthdr, >>> optlen); >>> - unfragpartlen = optlen + sizeof(struct ip6_hdr); >>> - >>> /* >>> * NOTE: we don't add AH/ESP length here (done in >>> * ip6_ipsec_output()). >>> @@ -508,6 +505,8 @@ ip6_output(struct mbuf *m0, struct ip6_pktopts >>> *opt, >>> MAKE_EXTHDR(opt->ip6po_dest2, &exthdrs.ip6e_dest2, >>> optlen); >>> } >>> + unfragpartlen = optlen + sizeof(struct ip6_hdr); >>> + >>> /* >>> * If there is at least one extension header, >>> * separate IP6 header from the payload. >> > And with this patch the test passes again. And fails for other packets. The patch is wrong. Offset gets changed after we set unfragpartlen inside the block so moving it outside the block unfragpartlen may point to the wrong thing. Your tests are too simple to detect this problem. Try this one please: Index: ip6_output.c =================================================================== --- ip6_output.c (revision 358297) +++ ip6_output.c (working copy) @@ -497,7 +497,7 @@ ip6_output(struct mbuf *m0, struct ip6_pktopts *op */ bzero(&exthdrs, sizeof(exthdrs)); optlen = 0; - unfragpartlen = 0; + unfragpartlen = sizeof(struct ip6_hdr); if (opt) { /* Hop-by-Hop options header. */ MAKE_EXTHDR(opt->ip6po_hbh, &exthdrs.ip6e_hbh, optlen); @@ -535,7 +535,7 @@ ip6_output(struct mbuf *m0, struct ip6_pktopts *op /* Routing header. */ MAKE_EXTHDR(opt->ip6po_rthdr, &exthdrs.ip6e_rthdr, optlen); - unfragpartlen = optlen + sizeof(struct ip6_hdr); + unfragpartlen += optlen; /* * NOTE: we don't add AH/ESP length here (done in /bz