Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Alexander Duyck <alexander.duyck@gmail.com>
To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: Netdev <netdev@vger.kernel.org>,
	David Miller <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Ido Schimmel <idosch@idosch.org>,
	chouhan.shreyansh630@gmail.com,
	Willem de Bruijn <willemb@google.com>
Subject: Re: [PATCH net] ip6_gre: validate csum_start only if CHECKSUM_PARTIAL
Date: Thu, 2 Sep 2021 13:27:33 -0700	[thread overview]
Message-ID: <CAKgT0Uc43z1qZr+wsB-UaxKV-3_RMjA3aw_LeeX-HswTvxitng@mail.gmail.com> (raw)
In-Reply-To: <20210902193447.94039-1-willemdebruijn.kernel@gmail.com>

On Thu, Sep 2, 2021 at 12:36 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> From: Willem de Bruijn <willemb@google.com>
>
> Only test integrity of csum_start if field is defined.
>
> With checksum offload and GRE tunnel checksum, gre_build_header will
> cheaply build the GRE checksum using local checksum offload. This
> depends on inner packet csum offload, and thus that csum_start points
> behind GRE. But validate this condition only with checksum offload.
>
> Link: https://lore.kernel.org/netdev/YS+h%2FtqCJJiQei+W@shredder/
> Fixes: 9cf448c200ba ("ip6_gre: add validation for csum_start")
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
>  net/ipv6/ip6_gre.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
> index 7baf41d160f5..c456bc7f7cdc 100644
> --- a/net/ipv6/ip6_gre.c
> +++ b/net/ipv6/ip6_gre.c
> @@ -629,8 +629,11 @@ static int gre_rcv(struct sk_buff *skb)
>
>  static int gre_handle_offloads(struct sk_buff *skb, bool csum)
>  {
> -       if (csum && skb_checksum_start(skb) < skb->data)
> +       /* Local checksum offload requires csum offload of the inner packet */
> +       if (csum && skb->ip_summed == CHECKSUM_PARTIAL &&
> +           skb_checksum_start(skb) < skb->data)
>                 return -EINVAL;
> +
>         return iptunnel_handle_offloads(skb,
>                                         csum ? SKB_GSO_GRE_CSUM : SKB_GSO_GRE);
>  }

Didn't see this come through until I had replied to the other patch.
Same comments apply here. The "csum" value probably doesn't need to be
checked when checking skb_checksum_start, and maybe this should
trigger a WARN_ON_ONCE since it should be rare and we should be fixing
any path that is requesting CHECKSUM_PARTIAL without setting
skb_checksum_start.

      parent reply	other threads:[~2021-09-02 20:27 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-02 19:34 Willem de Bruijn
2021-09-02 19:34 ` [PATCH net] ip_gre: " Willem de Bruijn
2021-09-02 20:25   ` Alexander Duyck
2021-09-02 20:29     ` Willem de Bruijn
2021-09-02 21:17       ` Alexander Duyck
2021-09-02 21:44         ` Willem de Bruijn
2021-09-03  2:01           ` Alexander Duyck
2021-09-03  2:40             ` Willem de Bruijn
2021-09-03 16:46               ` Alexander Duyck
2021-09-03 19:38                 ` Willem de Bruijn
2021-09-03 23:13                   ` Alexander Duyck
2021-09-04 14:45                     ` Willem de Bruijn
2021-09-04 15:35                       ` Alexander Duyck
2021-09-04 21:40                         ` Willem de Bruijn
2021-09-04 21:53                           ` Alexander Duyck
2021-09-04 22:05                             ` Willem de Bruijn
2021-09-04 23:47                               ` Alexander Duyck
2021-09-05 15:24                                 ` Willem de Bruijn
2021-09-05 15:53                                   ` Alexander Duyck
2021-09-06  3:02                                     ` Willem de Bruijn
2021-09-02 20:27 ` Alexander Duyck [this message]

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=CAKgT0Uc43z1qZr+wsB-UaxKV-3_RMjA3aw_LeeX-HswTvxitng@mail.gmail.com \
    --to=alexander.duyck@gmail.com \
    --cc=chouhan.shreyansh630@gmail.com \
    --cc=davem@davemloft.net \
    --cc=idosch@idosch.org \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=willemb@google.com \
    --cc=willemdebruijn.kernel@gmail.com \
    --subject='Re: [PATCH net] ip6_gre: validate csum_start only if CHECKSUM_PARTIAL' \
    /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

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).