netdev - Re: [PATCH] net: always use icmp{,v6}_ndo_send from ndo_start

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: <CAHmME9o2yPQ+Ai12XcCjF3jMVcMT_aooFCeKkfgFFOnqPmK_yg@mail.gmail.com>
Date:   Fri, 26 Feb 2021 23:22:45 +0100
From:   "Jason A. Donenfeld" <Jason@...c4.com>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc:     Network Development <netdev@...r.kernel.org>
Subject: Re: [PATCH] net: always use icmp{,v6}_ndo_send from ndo_start_xmit

On Fri, Feb 26, 2021 at 10:25 PM Willem de Bruijn
<willemdebruijn.kernel@...il.com> wrote:
>
> On Thu, Feb 25, 2021 at 6:46 PM Jason A. Donenfeld <Jason@...c4.com> wrote:
> >
> > There were a few remaining tunnel drivers that didn't receive the prior
> > conversion to icmp{,v6}_ndo_send. Knowing now that this could lead to
> > memory corrution (see ee576c47db60 ("net: icmp: pass zeroed opts from
> > icmp{,v6}_ndo_send before sending") for details), there's even more
> > imperative to have these all converted. So this commit goes through the
> > remaining cases that I could find and does a boring translation to the
> > ndo variety.
> >
> > Cc: Willem de Bruijn <willemb@...gle.com>
> > Signed-off-by: Jason A. Donenfeld <Jason@...c4.com>
>
> Using a stack variable over skb->cb[] is definitely the right fix for
> all of these. Thanks Jason.
>
> Only part that I don't fully know is the conntrack conversion. That is
> a behavioral change. What is the context behind that? I assume it's
> fine. In that if needed, that is the case for all devices, nothing
> specific about the couple that call icmp(v6)_ndo_send already.

That's actually a sensible change anyway. icmp_send does something
bogus if the packet has already passed through netfilter, which is why
the ndo variant was adopted. So it's good and correct for these to
change in that way.

Jason