netdev - Re: [PATCHv3 net] bonding: fix ICMPv6 header handling when receiving IPv6 messages

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: <24154.1668715451@famine>
Date:   Thu, 17 Nov 2022 12:04:11 -0800
From:   Jay Vosburgh <jay.vosburgh@...onical.com>
To:     Hangbin Liu <liuhangbin@...il.com>
cc:     "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <eric.dumazet@...il.com>, netdev@...r.kernel.org,
        Jakub Kicinski <kuba@...nel.org>,
        Jonathan Toppins <jtoppins@...hat.com>,
        Paolo Abeni <pabeni@...hat.com>,
        David Ahern <dsahern@...il.com>, Liang Li <liali@...hat.com>,
        David Ahern <dsahern@...nel.org>
Subject: Re: [PATCHv3 net] bonding: fix ICMPv6 header handling when receiving IPv6 messages

Hangbin Liu <liuhangbin@...il.com> wrote:

>On Wed, Nov 16, 2022 at 08:29:58PM -0800, Jay Vosburgh wrote:
>> > #if IS_ENABLED(CONFIG_IPV6)
>> >-	} else if (is_ipv6) {
>> >+	} else if (is_ipv6 && skb_header_pointer(skb, 0, sizeof(ip6_hdr), &ip6_hdr)) {
>> > 		return bond_na_rcv(skb, bond, slave);
>> > #endif
>> > 	} else {
>> >
>> >What do you think?
>> 
>> 	I don't see how this solves the icmp6_hdr() / ipv6_hdr() problem
>> in bond_na_rcv(); skb_header_pointer() doesn't do a pull, it just copies
>> into the supplied struct (if necessary).
>
>Hmm... Maybe I didn't get what you and Eric means. If we can copy the
>supplied buffer success, doesn't this make sure IPv6 header is in skb?

	The header is in the skb, but it may not be in the linear part
of the skb, i.e., the header is wholly or partially in a skb frag, not
in the area covered by skb->data ... skb->tail.  The various *_hdr()
macros only look in the linear area, not the frags, and don't check to
see if the linear area contains the entire header.

	skb_header_pointer() is smart enough to check, and if the
requested data is entirely within the linear area, it returns a pointer
to there; if not, it copies from the frags into the supplied struct and
returns a pointer to that.  What it doesn't do is a pull (move data from
a frag into the linear area), so merely calling skb_header_pointer()
doesn't affect the layout of what's in the skb (which is the point,
bonding uses it here to avoid changing the skb).

	There may be better explanations out there, but

http://vger.kernel.org/~davem/skb_data.html

	covers the basics.  Look for the references to "paged data."

	-J

---
	-Jay Vosburgh, jay.vosburgh@...onical.com