netdev - Re: IP oversized ip oacket from - header size should be skipped?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAA85sZsa2wuFAn+Ad3zJR+NaXKcO5zUAx7=0M5KE4dX-3wWgKw@mail.gmail.com>
Date: Sun, 30 Jun 2024 11:49:51 +0200
From: Ian Kumlien <ian.kumlien@...il.com>
To: Florian Westphal <fw@...len.de>
Cc: Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: IP oversized ip oacket from - header size should be skipped?

On Sun, Jun 30, 2024 at 1:51 AM Ian Kumlien <ian.kumlien@...il.com> wrote:
>
> So, yeaj, caffeine induced thinking, been reading RFC:s and yes, it's
> completely correct that fragment ip headers should be skipped
>
> completely logical as well, what does confuse me though is that i can
> get thousands of:
> [ 1415.631438] IPv4: Oversized IP packet from <local ip>
>
> I did change to get the size, and
> --- a/net/ipv4/ip_fragment.c
> +++ b/net/ipv4/ip_fragment.c
> @@ -474,7 +474,7 @@ static int ip_frag_reasm(struct ipq *qp, struct
> sk_buff *skb,
>         err = -ENOMEM;
>         goto out_fail;
>  out_oversize:
> -       net_info_ratelimited("Oversized IP packet from %pI4\n",
> &qp->q.key.v4.saddr);
> +       net_info_ratelimited("Oversized IP packet from %pI4 %i >
> 65535\n", &qp->q.key.v4.saddr, len);
>  out_fail:
>         __IP_INC_STATS(net, IPSTATS_MIB_REASMFAILS);
>         return err;
>
> Yields:  66260 > 65535
>
> Which is constantly 725 bytes too large, assumed to be ~16 bytes per packet
>
> Checking the calculation quickly becomes beyond me
>         /* Determine the position of this fragment. */
>         end = offset + skb->len - skb_network_offset(skb) - ihl;
>
> Since skb_network_offset(skb) expands to:
> (skb->head + skb->network_header) - skb->data
>
> And you go, oh... heck ;)
>
> I just find it weird that localhost can generate a packet (without raw
> or xdp) that is oversize, I'll continue checking
> (once you've started making a fool of yourself, no reason to stop =))
>

So continued with:
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -474,7 +474,24 @@ static int ip_frag_reasm(struct ipq *qp, struct
sk_buff *skb,
        err = -ENOMEM;
        goto out_fail;
 out_oversize:
-       net_info_ratelimited("Oversized IP packet from %pI4\n",
&qp->q.key.v4.saddr);
+       net_info_ratelimited("Oversized IP packet from %pI4 %u >
65535\n", &qp->q.key.v4.saddr, len);
+       {
+               struct rb_node *p = rb_first(&qp->q.rb_fragments);
+               unsigned int frg_cnt = 0;
+               int offset, ihl, flags;
+
+               while (p) {
+                       struct sk_buff *skb = rb_entry(p, struct
sk_buff, rbnode);
+
+                       offset = ntohs(ip_hdr(skb)->frag_off);
+                       flags = offset & ~IP_OFFSET;
+                       offset &= IP_OFFSET;
+                       offset <<= 3;
+                       ihl = ip_hdrlen(skb);
+                       printk("FRAGMENT %u: skb->truesize = %u,
skb->len = %u, len - skb_network_offset = %u -- end = %u final? %s\n",
frg_cnt++, skb->truesize, skb->len, skb->len -
skb_network_offset(skb), offset + skb->len - skb_network_offset(skb) -
ihl, ((flags & IP_MF) == 0) ? "yes" : "no");
+                       p = rb_next(p);
+               }
+       }
 out_fail:
        __IP_INC_STATS(net, IPSTATS_MIB_REASMFAILS);
        return err;

And the result is:
[ 1764.949410] FRAGMENT 0: skb->truesize = 2304, skb->len = 1472, len
- skb_network_offset = 1492 -- end = 1472 final? no

I never seem to get more than one fragment so now this feels more like
an uninitialized variable?