Re: Calculation of IPv6 fragmented part length

[Ryota Ozaki <ozaki-r%netbsd.org@localhost>]

Hi,

On Mon, Apr 15, 2024 at 12:24 AM Yasuyuki KOZAKAI
<yasuyuki.kozakai%gmail.com@localhost> wrote:
>
> Hello,
>
> The current kernel tries to calculate the fragmented part length by ntohs(ip6->ip6_plen) - offset) and it checks if it is zero, but the calculation is incorrect.
>
> Then the current kernel drops a IPv6 packet which consists of 40 octets IPv6 header, 8 octets Fragment Header, and 32 octets fragmented part.
>
> The following changes will resolve the problem.
> Please review & consider to apply it.
>
> I did not import the code from FreeBSD to resolve it, because FreeBSD checks zero length fragment after handling atomic fragment.
> FreeBSD would pass a zero length fragment with M flag = 0 and Fragment Offset = 0 to upper layer processing ( I did not test it, though).
> It would be better to drop such packet without passing it to upper layer.
>
> Best regards,

Thank you for the report. The patch looks correct to me.
I'll commit it.

Just curious, how did you find the issue? It seems to rarely occur.

Thanks,
  ozaki-r

>
>
> Index: sys/netinet6/frag6.c
> ===================================================================
> RCS file: /cvsroot/src/sys/netinet6/frag6.c,v
> retrieving revision 1.77
> diff -u -p -r1.77 frag6.c
> --- sys/netinet6/frag6.c        29 Aug 2023 17:01:35 -0000      1.77
> +++ sys/netinet6/frag6.c        14 Apr 2024 13:42:43 -0000
> @@ -206,9 +206,11 @@ frag6_input(struct mbuf **mp, int *offp,
>          * sizeof(struct ip6_frag) == 8
>          * sizeof(struct ip6_hdr) = 40
>          */
> -       if ((ip6f->ip6f_offlg & IP6F_MORE_FRAG) &&
> -           (((ntohs(ip6->ip6_plen) - offset) == 0) ||
> -            ((ntohs(ip6->ip6_plen) - offset) & 0x7) != 0)) {
> +       frgpartlen = sizeof(struct ip6_hdr) + ntohs(ip6->ip6_plen)
> +                       - offset - sizeof(struct ip6_frag);
> +       if ((frgpartlen == 0) ||
> +           ((ip6f->ip6f_offlg & IP6F_MORE_FRAG) &&
> +            (frgpartlen & 0x7) != 0)) {
>                 icmp6_error(m, ICMP6_PARAM_PROB, ICMP6_PARAMPROB_HEADER,
>                     offsetof(struct ip6_hdr, ip6_plen));
>                 in6_ifstat_inc(dstifp, ifs6_reass_fail);
> @@ -316,7 +318,6 @@ frag6_input(struct mbuf **mp, int *offp,
>          * in size. If it would exceed, discard the fragment and return an
>          * ICMP error.
>          */
> -       frgpartlen = sizeof(struct ip6_hdr) + ntohs(ip6->ip6_plen) - offset;
>         if (q6->ip6q_unfrglen >= 0) {
>                 /* The 1st fragment has already arrived. */
>                 if (q6->ip6q_unfrglen + fragoff + frgpartlen > IPV6_MAXPACKET) {
>