netdev - Re: [PATCH] Fix the upper MTU limit in ipv6 GRE tunnel

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: <20131007020218.GF9295@order.stressinduktion.org>
Date:	Mon, 7 Oct 2013 04:02:18 +0200
From:	Hannes Frederic Sowa <hannes@...essinduktion.org>
To:	Oussama Ghorbel <ou.ghorbel@...il.com>
Cc:	"David S. Miller" <davem@...emloft.net>,
	Alexey Kuznetsov <kuznet@....inr.ac.ru>,
	James Morris <jmorris@...ei.org>,
	Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
	Patrick McHardy <kaber@...sh.net>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Fix the upper MTU limit in ipv6 GRE tunnel

On Sun, Oct 06, 2013 at 08:18:15PM +0100, Oussama Ghorbel wrote:
> Yes, to summarize, the idea of this patch was to fix the incoherence
> in the condition of ip6gre_tunnel_change_mtu function
> 
>   if (new_mtu < 68 ||
>    new_mtu > 0xFFF8 - dev->hard_header_len - tunnel->hlen)
> 
> From the ip6gre_tnl_link_config function we can see that:
> The variable addend is equal the ipv6 header + gre header (including
> the gre options)
> On the other hand hard_header_len equal to the header of the lower
> layer + addend.
> So the quantity - (dev->hard_header_len + tunnel->hlen) equals - (eth
> header + ipv6 header + gre header + ipv6 header + gre header) which by
> no means this would represent anything!  (I've just taken ipv6 over
> ethernet as example)
> 
> As we have seen there is another approach to fix this issue is to
> re-factor the hlen to hold only the length of gre as it's done for
> ipv4 gre, however the solution provided in the patch seems to be
> regression risk-less.

I agree, it actually does not worsen the situation:

Acked-by: Hannes Frederic Sowa <hannes@...essinduktion.org>

> Although the value hold by hlen is not coherent with the variable name
> nor with ipv4, I think there is an advantage of the current approach
> of ipv6 hlen over ipv4 hlen, because we save the calculation of ipv6
> header each time. Ex:
> In ipv4 gre and in the function ipgre_header:
> iph = (struct iphdr *)skb_push(skb, t->hlen + sizeof(*iph));
> In ipv6 and in the function ip6gre_header
> ipv6h = (struct ipv6hdr *)skb_push(skb, t->hlen);

I see your point. But we should take care that t->hlen is always initialized,
regardless if we got a route and outgoing device or not.

Greetings,

  Hannes

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html