netdev - Re: [PATCH] net: ip_finish_output_gso: If skb_gso_network_seglen exceeds MTU, do segmentation even for non IPSKB

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: <20160714171333.00657367@pixies>
Date:	Thu, 14 Jul 2016 17:13:33 +0300
From:	Shmulik Ladkani <shmulik.ladkani@...ellosystems.com>
To:	Hannes Frederic Sowa <hannes@...essinduktion.org>,
	Florian Westphal <fw@...len.de>
Cc:	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, shmulik.ladkani@...il.com,
	netdev@...r.kernel.org,
	Alexander Duyck <alexander.duyck@...il.com>,
	Tom Herbert <tom@...bertland.com>
Subject: Re: [PATCH] net: ip_finish_output_gso: If skb_gso_network_seglen
 exceeds MTU, do segmentation even for non IPSKB_FORWARDED skbs

Hi,

On Thu, 14 Jul 2016 15:12:07 +0200, hannes@...essinduktion.org wrote:
> I liked the fact that setting IPSKB_FORWARDED was only contained in
> vxlan and as such wouldn't have as much impact. It was more logically
> easy to review for me actually.

I agree here. It is rather safe and to the point.

I'm trying to exaust other alternatives because it has one potential
drawback: the name IPSKB_FORWARDED suggests ipv4 forwarding had
happened. Indeed, current setters of IPSKB_FORWARDED are ip_forward and
ip_mr_forward.

If we set IPSKB_FORWARDED in iptunnel_xmit, with packet not being ipv4
forwarded (e.g. bridged from some ingress device to a tunnel device), it
presents a nuance whose impact is yet to be determined.

For example, what about a packet that gets encapsulated and sent to a
multicast destination? The condition controlling mc loop-back in
ip_mc_output is affected by the flag.

> > Which ensures only the following conditions go to the expensive
> > skb_gso_validate_mtu:
> > 
> > 1. IPSKB_FORWARDED is on
> > 2. IPSKB_FORWARDED is off, but sk exists and gso_size is untrusted.
> >    Meaning: we have a packet arriving from higher layers (sk is set)
> >    with a gso_size out of host's control.
> 
> When can this really happen? In general we don't want to refragment gso
> skb's and I think we can only make an exception for vxlan or udp.

When IPSKB_FORWARDED is off, we'll get SKB_GSO_DODGY if packet
originally arrived from tap/macvtap/packet and it did NOT pass ipv4
forwarding (e.g bridges: tap0 to eth0 bridge, or tap0 to vxlan0 bridge).

The rationale: in the SKB_GSO_DODGY cases, the gso_size is given by
the user's virtio-net header, which is not in kernel's control.

This exactly resembles the usecase: tap0 gives packets with gso_size
unsuitable for encapsulation and segmentation. I have no control on
the source that gives those packets.

If (1) it does not make sense, or (2) considered too broad-spectrum to
asses, then we can go with the safer IPSKB_FORWARDED approach.

Let me know.

Regards,
Shmulik