netdev - Re: [PATCH net-next 09/10] netfilter: get ipv6 pktlen properly in length

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <7181c170-3d10-fd3a-2769-7e8e40d54b41@gmail.com>
Date:   Sun, 15 Jan 2023 16:57:36 -0700
From:   David Ahern <dsahern@...il.com>
To:     Xin Long <lucien.xin@...il.com>, Eric Dumazet <edumazet@...gle.com>
Cc:     network dev <netdev@...r.kernel.org>, davem@...emloft.net,
        kuba@...nel.org, Paolo Abeni <pabeni@...hat.com>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        Pravin B Shelar <pshelar@....org>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Jiri Pirko <jiri@...nulli.us>,
        Pablo Neira Ayuso <pablo@...filter.org>,
        Florian Westphal <fw@...len.de>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        Ilya Maximets <i.maximets@....org>,
        Aaron Conole <aconole@...hat.com>,
        Roopa Prabhu <roopa@...dia.com>,
        Nikolay Aleksandrov <razor@...ckwall.org>,
        Mahesh Bandewar <maheshb@...gle.com>,
        Paul Moore <paul@...l-moore.com>,
        Guillaume Nault <gnault@...hat.com>
Subject: Re: [PATCH net-next 09/10] netfilter: get ipv6 pktlen properly in
 length_mt6

On 1/15/23 1:14 PM, Xin Long wrote:
>>
>> So skb->len is not the root of trust. Some transport mediums might add paddings.
>>

That padding is to meet minimum packet sizes AIUI.

Ethernet for example has a minimum packet length of 60B. Some protocols
running over ethernet can generate packets less than 60B. For example,
ARP is 52B as I recall so something in the hardware pipeline must pad
that out to 60B. IPv4 with UDP and small L4 payloads is another example.
So the trim is to remove the padding added to meet a minimum size. That
should not be relevant for large packets.

>> Ipv4 has a similar logic in ip_rcv_core().
>>
>> len = ntohs(iph->tot_len);
>> if (skb->len < len) {
>>      drop_reason = SKB_DROP_REASON_PKT_TOO_SMALL;
>>      __IP_INC_STATS(net, IPSTATS_MIB_INTRUNCATEDPKTS);
>>     goto drop;
>> } else if (len < (iph->ihl*4))
>>      goto inhdr_error;
>>
>> /* Our transport medium may have padded the buffer out. Now we know it
>> * is IP we can trim to the true length of the frame.
>> * Note this now means skb->len holds ntohs(iph->tot_len).
>> */
>> if (pskb_trim_rcsum(skb, len)) {
>>       __IP_INC_STATS(net, IPSTATS_MIB_INDISCARDS);
>>       goto drop;
>> }
>>
>> After your changes, we might accept illegal packets that were properly
>> dropped before.
> I think skb->len is trustable for GSO/GRO packets.

That is my understanding as well.

> In ipv6_gro_complete/inet_gro_complete():
> The new length for payload_len or iph->tot_len are all calculated from skb->len.
> As I said in the cover-letter, "there is no padding in GSO/GRO packets".
> Or am I missing something?
> 
> Thanks.