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: <CA+FuTSdCnCKFrpe-G55rPCq_D7uv4EaQ4z8XW2MOtTRKcWVJYQ@mail.gmail.com>
Date:   Fri, 26 Feb 2021 18:28:56 -0500
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     "Jason A. Donenfeld" <Jason@...c4.com>
Cc:     Willem de Bruijn <willemdebruijn.kernel@...il.com>,
        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 5:39 PM Jason A. Donenfeld <Jason@...c4.com> wrote:
>
> 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

Something bogus, how? Does this apply to all uses of conntrack?
Specifically NAT? Not trying to be obtuse, but I really find it hard
to evaluate that part.

Please cc: the maintainers for patches that are meant to be merged, btw.