linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To: Ido Schimmel <idosch@idosch.org>
Cc: Willem de Bruijn <willemdebruijn.kernel@gmail.com>,
	Shreyansh Chouhan <chouhan.shreyansh630@gmail.com>,
	davem@davemloft.net, yoshfuji@linux-ipv6.org, dsahern@kernel.org,
	kuba@kernel.org, pshelar@nicira.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	syzbot+ff8e1b9f2f36481e2efc@syzkaller.appspotmail.com,
	Edward Cree <ecree@solarflare.com>
Subject: Re: [PATCH 1/2 net] ip_gre: add validation for csum_start
Date: Wed, 1 Sep 2021 17:39:45 -0400	[thread overview]
Message-ID: <CA+FuTScx8cCQEOqsmj1eazMkRPqfb-EaqrqH+kmS_sKCFfr7kg@mail.gmail.com> (raw)
In-Reply-To: <YS+h/tqCJJiQei+W@shredder>

On Wed, Sep 1, 2021 at 11:53 AM Ido Schimmel <idosch@idosch.org> wrote:
>
> Thanks for the quick reply, Willem.
>
> On Wed, Sep 01, 2021 at 09:46:48AM -0400, Willem de Bruijn wrote:
> > Thanks for the detailed report, Ido.
> >
> > This is a gre tunnel device with csum/ocsum enabled, correct?
>
> Correct.
>
> >
> > How was this packet generated: does it come from the local stack or is
> > it a custom packet injected from userspace, e.g., with a packet socket
> > with vnet_hdr?
>
> The packet is received by a physical port and injected to the kernel's
> Rx path by mlxsw (which does not support checksumming). The IPv4 routing
> code then forwards the packet to the GRE tunnel.
>
> I was able to reproduce the issue using veth pairs and a packet socket
> [1]. Running the reproducer with the debug patch from before, I get the
> following output [2].

Thanks for that device independent repro.

As expected, the following fixes it for these packets:

-       if (csum && skb_checksum_start(skb) < skb->data)
+       if (csum && skb->ip_summed == CHECKSUM_PARTIAL &&
+           skb_checksum_start(skb) < skb->data)

The question is whether we're doing the right thing when CHECKSUM_PARTIAL
is set.

Local checksum offload allows for cheap calculation of outer checksums, by
relying on the fact that the inner packet with the checksum field filled in will
sum to zero. It relies on checksum offload to compute this inner checksum,
so expects csum_start and csum_off to point after the GRE header.

If so, then the existing fix won't break correctly configured skbs as it only
drops packets for which this does not hold.

  reply	other threads:[~2021-09-01 21:40 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-19 14:34 [PATCH] ip_gre/ip6_gre: add check for invalid csum_start Shreyansh Chouhan
2021-08-19 16:56 ` Willem de Bruijn
2021-08-19 17:04   ` Jakub Kicinski
2021-08-21  7:14     ` [PATCH 1/2 net] ip_gre: add validation for csum_start Shreyansh Chouhan
2021-08-21 13:41       ` Willem de Bruijn
2021-09-01 11:53         ` Ido Schimmel
2021-09-01 13:46           ` Willem de Bruijn
2021-09-01 15:53             ` Ido Schimmel
2021-09-01 21:39               ` Willem de Bruijn [this message]
2021-08-22 20:30       ` patchwork-bot+netdevbpf
2021-08-21  7:14     ` [PATCH 2/2 net] ip6_gre: " Shreyansh Chouhan
2021-08-21 13:42       ` Willem de Bruijn
2021-08-21  7:18     ` [PATCH] ip_gre/ip6_gre: add check for invalid csum_start Shreyansh Chouhan
2021-08-21 13:44       ` Willem de Bruijn

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=CA+FuTScx8cCQEOqsmj1eazMkRPqfb-EaqrqH+kmS_sKCFfr7kg@mail.gmail.com \
    --to=willemdebruijn.kernel@gmail.com \
    --cc=chouhan.shreyansh630@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=ecree@solarflare.com \
    --cc=idosch@idosch.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pshelar@nicira.com \
    --cc=syzbot+ff8e1b9f2f36481e2efc@syzkaller.appspotmail.com \
    --cc=yoshfuji@linux-ipv6.org \
    /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).