From: Xie He <xie.he.0141@gmail.com>
To: Cong Wang <xiyou.wangcong@gmail.com>
Cc: Linux Kernel Network Developers <netdev@vger.kernel.org>,
syzbot <syzbot+4a2c52677a8a1aa283cb@syzkaller.appspotmail.com>,
William Tu <u9012063@gmail.com>,
Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Subject: Re: [Patch net v2] ip_gre: set dev->hard_header_len and dev->needed_headroom properly
Date: Sun, 11 Oct 2020 13:32:35 -0700 [thread overview]
Message-ID: <CAJht_EP5LWUadxwMpdsRAhUrjaUHpi-1QO5N28r7Sqtp4Qxjpw@mail.gmail.com> (raw)
In-Reply-To: <20201011191129.991-1-xiyou.wangcong@gmail.com>
On Sun, Oct 11, 2020 at 12:11 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> GRE tunnel has its own header_ops, ipgre_header_ops, and sets it
> conditionally. When it is set, it assumes the outer IP header is
> already created before ipgre_xmit().
>
> This is not true when we send packets through a raw packet socket,
> where L2 headers are supposed to be constructed by user. Packet
> socket calls dev_validate_header() to validate the header. But
> GRE tunnel does not set dev->hard_header_len, so that check can
> be simply bypassed, therefore uninit memory could be passed down
> to ipgre_xmit(). Similar for dev->needed_headroom.
>
> dev->hard_header_len is supposed to be the length of the header
> created by dev->header_ops->create(), so it should be used whenever
> header_ops is set, and dev->needed_headroom should be used when it
> is not set.
Hi, thanks for attempting to fix this tunnel. Are we still considering
removing header_ops->create?
As said in my email sent previously today, I want to remove
header_ops->create because 1) this keeps the un-exposed headers of GRE
devices consistent with those of GRETAP devices, and 2) I think the
GRE header (and the headers before the GRE header) is not actually the
L2 header of the tunnel (the Wikipedia page for "Generic Routing
Encapsulation" doesn't consider this protocol to be at L2 either).
I'm not sure if you still agree to remove header_ops->create. Do you
still agree but think it'd be better to do that in a separate patch?
Removing header_ops->create would simplify the fixing of the issue you
are trying to fix, too, because that way we would no longer need to
use header_ops or hard_header_len. Also, I'm worried that changing
hard_header_len (or needed_headroom) in ipgre_link_update would have
racing issues. If we remove header_ops, we no longer need to use
hard_header_len and we can just set needed_headroom to the maximum
value, so that we no longer need to update them in ipgre_link_update.
next prev parent reply other threads:[~2020-10-11 20:32 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-11 19:11 [Patch net v2] ip_gre: set dev->hard_header_len and dev->needed_headroom properly Cong Wang
2020-10-11 20:32 ` Xie He [this message]
2020-10-11 21:06 ` Willem de Bruijn
2020-10-11 21:50 ` Xie He
2020-10-11 21:00 ` Willem de Bruijn
2020-10-11 21:25 ` Willem de Bruijn
2020-10-11 22:03 ` Xie He
2020-10-14 8:51 ` Xie He
2020-10-14 15:12 ` Willem de Bruijn
2020-10-14 19:47 ` Xie He
2020-10-14 20:19 ` Willem de Bruijn
2020-10-15 1:38 ` Xie He
2020-10-15 2:24 ` Xie He
2020-10-15 13:42 ` Willem de Bruijn
2020-10-15 19:19 ` Xie He
2020-10-15 19:56 ` Willem de Bruijn
2020-10-11 22:45 ` Xie He
2020-10-12 22:26 ` Cong Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAJht_EP5LWUadxwMpdsRAhUrjaUHpi-1QO5N28r7Sqtp4Qxjpw@mail.gmail.com \
--to=xie.he.0141@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=syzbot+4a2c52677a8a1aa283cb@syzkaller.appspotmail.com \
--cc=u9012063@gmail.com \
--cc=willemdebruijn.kernel@gmail.com \
--cc=xiyou.wangcong@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).