netdev - Re: [PATCH net-next 7/8] net: ip_tunnel: remove 'csum_help' argument to iptunnel_handle

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: <CALx6S34MRoLw_JiHuyDANwoF9+u7k9R-UDb_RGP+L-d9D=f3WQ@mail.gmail.com>
Date:	Fri, 8 Jan 2016 16:44:14 -0800
From:	Tom Herbert <tom@...bertland.com>
To:	Alexander Duyck <alexander.duyck@...il.com>
Cc:	Edward Cree <ecree@...arflare.com>,
	David Miller <davem@...emloft.net>,
	Netdev <netdev@...r.kernel.org>, linux-net-drivers@...arflare.com
Subject: Re: [PATCH net-next 7/8] net: ip_tunnel: remove 'csum_help' argument
 to iptunnel_handle_offloads

On Fri, Jan 8, 2016 at 4:35 PM, Alexander Duyck
<alexander.duyck@...il.com> wrote:
> On Fri, Jan 8, 2016 at 11:47 AM, Edward Cree <ecree@...arflare.com> wrote:
>> All users now pass false, so we can remove it, and remove the code that
>>  was conditional upon it.
>>
>> Signed-off-by: Edward Cree <ecree@...arflare.com>
>> ---
>>  drivers/net/vxlan.c             |  4 ++--
>>  include/net/ip_tunnels.h        |  3 +--
>>  include/net/udp_tunnel.h        |  3 +--
>>  net/ipv4/fou.c                  |  4 ++--
>>  net/ipv4/ip_gre.c               |  3 +--
>>  net/ipv4/ip_tunnel_core.c       | 15 +--------------
>>  net/ipv4/ipip.c                 |  2 +-
>>  net/ipv6/sit.c                  |  4 ++--
>>  net/netfilter/ipvs/ip_vs_xmit.c |  6 ++----
>>  9 files changed, 13 insertions(+), 31 deletions(-)
>>
>
>
>> diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
>> index 1db8418..f98bd53 100644
>> --- a/net/ipv4/ip_tunnel_core.c
>> +++ b/net/ipv4/ip_tunnel_core.c
>> @@ -147,7 +147,6 @@ struct metadata_dst *iptunnel_metadata_reply(struct metadata_dst *md,
>>  EXPORT_SYMBOL_GPL(iptunnel_metadata_reply);
>>
>>  struct sk_buff *iptunnel_handle_offloads(struct sk_buff *skb,
>> -                                        bool csum_help,
>>                                          int gso_type_mask)
>>  {
>>         int err;
>> @@ -165,19 +164,7 @@ struct sk_buff *iptunnel_handle_offloads(struct sk_buff *skb,
>>                 return skb;
>>         }
>>
>> -       /* If packet is not gso and we are resolving any partial checksum,
>> -        * clear encapsulation flag. This allows setting CHECKSUM_PARTIAL
>> -        * on the outer header without confusing devices that implement
>> -        * NETIF_F_IP_CSUM with encapsulation.
>> -        */
>> -       if (csum_help)
>> -               skb->encapsulation = 0;
>> -
>> -       if (skb->ip_summed == CHECKSUM_PARTIAL && csum_help) {
>> -               err = skb_checksum_help(skb);
>> -               if (unlikely(err))
>> -                       goto error;
>> -       } else if (skb->ip_summed != CHECKSUM_PARTIAL)
>> +       if (skb->ip_summed != CHECKSUM_PARTIAL)
>>                 skb->ip_summed = CHECKSUM_NONE;
>>
>
> So this patch is a bit broken here.  We should be clearing
> skb->encapsulation if CHECKSUM_PARTIAL is not set.  That way we don't
> incorrectly pull in the inner headers when computing the outer
> checksum.
>
In the original code this is done in csum_help argument is true in
iptunnel_handle_offloads. These patches essentially imply that
csum_help would always be false so we shouldn't need to clear
skb->encapsulation any more?

Tom

> - Alex