netdev - Re: [Patch net v2] ip_gre: set dev->hard_header_len and dev->needed

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]

Date:   Sun, 11 Oct 2020 14:50:00 -0700
From:   Xie He <xie.he.0141@...il.com>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc:     Cong Wang <xiyou.wangcong@...il.com>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        syzbot <syzbot+4a2c52677a8a1aa283cb@...kaller.appspotmail.com>,
        William Tu <u9012063@...il.com>
Subject: Re: [Patch net v2] ip_gre: set dev->hard_header_len and
 dev->needed_headroom properly

On Sun, Oct 11, 2020 at 2:07 PM Willem de Bruijn
<willemdebruijn.kernel@...il.com> wrote:
>
> On Sun, Oct 11, 2020 at 4:42 PM Xie He <xie.he.0141@...il.com> wrote:
> >
> > 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.
>
> Our messages crossed.
>
> It seems there are legacy expectations that sendto/recvfrom packet
> sockets allow writing/reading the outer IP address, as of commit
> 6a5f44d7a048 ("[IPV4] ip_gre: sendto/recvfrom NBMA address"). That is
> the express purpose of that commit.
>
> The behavior is inconsistent with other tunnels, as you also point
> out, and probably only rarely used if at all. I would love to get rid
> of it, but given that we cannot be certain that it is unused, I'm afraid
> that we have to continue to support this special case.

OK. At least we agree that in an ideal world header_ops for GRE
devices should be removed.

Thanks, Willem.