[PATCH net 0/5] net: avoid truncated transport header offsets

[PATCH net 0/5] net: avoid truncated transport header offsets

[Eric Dumazet]

skb->transport_header is a 16-bit offset from skb->head, and U16_MAX
is reserved as the "not set" value. skb_set_transport_header() silently
truncates an offset that does not fit.

The first patch addresses the report from Hyunwoo Kim, by applying
the IP6_MAX_EXT_HDRS_CNT limit to ipv6_gro_pull_exthdrs().

The four following patches convert places storing an offset derived
from an IPv6 extension header walk to skb_set_transport_header_careful().
A single chain being bounded to 24576 bytes, these need a large
skb->data - skb->head to overflow, for instance after nested
decapsulations. They are hardening, no reproducer was written.

The same change for nf_ct_frag6_gather() is sent separately to
the netfilter tree.

Eric Dumazet (5):
  ipv6: gro: limit the number of extension headers
  ipv6: mcast: use skb_set_transport_header_careful() in
    ipv6_mc_check_exthdrs()
  net: openvswitch: use skb_set_transport_header_careful() in
    parse_ipv6hdr()
  sctp: use skb_set_transport_header_careful() in sctp_v6_err()
  udp: use skb_set_transport_header_careful() in __udp6_lib_err_encap()

 net/ipv6/ip6_offload.c | 8 ++++++++
 net/ipv6/mcast_snoop.c | 3 ++-
 net/ipv6/udp.c         | 7 ++++---
 net/openvswitch/flow.c | 3 ++-
 net/sctp/ipv6.c        | 5 ++++-
 5 files changed, 20 insertions(+), 6 deletions(-)

-- 
2.53.0

[PATCH net 1/5] ipv6: gro: limit the number of extension headers

[Eric Dumazet]

Hyunwoo Kim reported that a long chain of IPv6 extension headers
can push the transport header offset of a GRO packet beyond what
the 16-bit skb->transport_header can store. The truncated offset
is later used by gro_receive_network_flush() to locate the network
header of a packet held in the GRO list, leading to an out-of-bounds
read.

Since commit 3744b0964d52 ("ipv6: Implement limits on extension header
parsing"), ip6_protocol_deliver_rcu() drops packets carrying more than
IP6_MAX_EXT_HDRS_CNT extension headers, but ipv6_gro_pull_exthdrs()
was left unbounded.

There is no point trying to aggregate packets that the stack is going
to drop anyway if they are locally delivered. Apply the same limit in
ipv6_gro_pull_exthdrs().

When the limit is hit, @proto still designates an extension header.
Extension header offloads have no gro_receive() callback, so
ipv6_gro_receive() takes its existing "goto out" path with flush set,
and the packet is given to the normal receive path. Local delivery
drops it with SKB_DROP_REASON_IPV6_TOO_MANY_EXTHDRS, while forwarded
packets are simply not aggregated.

An extension header being at most 2048 bytes, this bounds a chain
to 24576 bytes.

Note that offsets can still accumulate over nested encapsulations,
so the places storing them should be hardened separately.

Fixes: 4b0ebbca3e16 ("net: gro: move L3 flush checks to tcp_gro_receive and udp_gro_receive_segment")
Cc: stable@vger.kernel.org
Reported-by: Hyunwoo Kim <imv4bel@gmail.com>
Link: https://lore.kernel.org/netdev/avTTQNyvwPP6LtGx@v4bel/
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David Ahern <dsahern@kernel.org>
Cc: Ido Schimmel <idosch@nvidia.com>
---
 net/ipv6/ip6_offload.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index 700f1861d8eb..01642900c0fa 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -26,6 +26,7 @@ static int ipv6_gro_pull_exthdrs(struct sk_buff *skb, int off, int proto)
 {
 	const struct net_offload *ops = NULL;
 	struct ipv6_opt_hdr *opth;
+	int exthdr_cnt = 0;
 
 	for (;;) {
 		int len;
@@ -38,6 +39,13 @@ static int ipv6_gro_pull_exthdrs(struct sk_buff *skb, int off, int proto)
 		if (!(ops->flags & INET6_PROTO_GSO_EXTHDR))
 			break;
 
+		/* Same limit as ip6_protocol_deliver_rcu().
+		 * @proto is left pointing to an extension header,
+		 * so that the caller does not aggregate this packet.
+		 */
+		if (unlikely(exthdr_cnt++ >= IP6_MAX_EXT_HDRS_CNT))
+			break;
+
 		opth = skb_gro_header(skb, off + sizeof(*opth), off);
 		if (unlikely(!opth))
 			break;
-- 
2.53.0

[PATCH net 2/5] ipv6: mcast: use skb_set_transport_header_careful() in ipv6_mc_check_exthdrs()

[Eric Dumazet]

skb->transport_header is a 16-bit offset from skb->head, and U16_MAX is
reserved as the "not set" value. skb_set_transport_header() silently
truncates an offset that does not fit.

ipv6_mc_check_exthdrs() stores the offset returned by ipv6_skip_exthdr(),
which depends on the extension headers found in the packet. Should the
value be truncated, ipv6_mc_check_icmpv6() and ipv6_mc_check_mld_msg()
would then validate and parse the MLD message at the wrong place.

Since commit 3744b0964d52 ("ipv6: Implement limits on extension header
parsing") a single chain is bounded to IP6_MAX_EXT_HDRS_CNT headers
(24576 bytes), so this needs a large skb->data - skb->head on top of it,
for instance after several nested decapsulations, each of them possibly
pulling its own extension header chain. No reproducer was written,
this is hardening.

Use skb_set_transport_header_careful() and treat such a packet as
broken (-EINVAL).

Fixes: 9afd85c9e455 ("net: Export IGMP/MLD message validation code")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 net/ipv6/mcast_snoop.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/mcast_snoop.c b/net/ipv6/mcast_snoop.c
index 04d5fcdfa6e0..7abf79815aa2 100644
--- a/net/ipv6/mcast_snoop.c
+++ b/net/ipv6/mcast_snoop.c
@@ -56,7 +56,8 @@ static int ipv6_mc_check_exthdrs(struct sk_buff *skb)
 	if (nexthdr != IPPROTO_ICMPV6)
 		return -ENOMSG;
 
-	skb_set_transport_header(skb, offset);
+	if (!skb_set_transport_header_careful(skb, offset))
+		return -EINVAL;
 
 	return 0;
 }
-- 
2.53.0

[PATCH net 3/5] net: openvswitch: use skb_set_transport_header_careful() in parse_ipv6hdr()

[Eric Dumazet]

skb->transport_header is a 16-bit offset from skb->head, and U16_MAX is
reserved as the "not set" value. skb_set_transport_header() silently
truncates an offset that does not fit.

parse_ipv6hdr() stores the payload offset found by ipv6_find_hdr(),
which depends on the extension headers found in the packet. Should the
value be truncated, key_extract_l3l4() would parse the L4 header at the
wrong place.

Since commit 3744b0964d52 ("ipv6: Implement limits on extension header
parsing") a single chain is bounded to IP6_MAX_EXT_HDRS_CNT headers
(24576 bytes), so this needs a large skb->data - skb->head on top of it,
for instance after several nested decapsulations, each of them possibly
pulling its own extension header chain. No reproducer was written,
this is hardening.

Use skb_set_transport_header_careful() and return -EPROTO if the offset
can not be stored. As for other ipv6_find_hdr() failures, the caller
then sets the transport header to the network header and does not
parse L4. While at it, use payload_ofs directly, since
nh_ofs + nh_len == payload_ofs.

Fixes: 1a37e412a022 ("net: Use 16bits for *_headers fields of struct skbuff")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 net/openvswitch/flow.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 3734416b518d..5fbeb0cca9dc 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -438,7 +438,8 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct sw_flow_key *key)
 		return -EPROTO;
 
 	nh_len = payload_ofs - nh_ofs;
-	skb_set_transport_header(skb, nh_ofs + nh_len);
+	if (unlikely(!skb_set_transport_header_careful(skb, payload_ofs)))
+		return -EPROTO;
 	key->ip.proto = nexthdr;
 	return nh_len;
 }
-- 
2.53.0

[PATCH net 4/5] sctp: use skb_set_transport_header_careful() in sctp_v6_err()

[Eric Dumazet]

skb->transport_header is a 16-bit offset from skb->head, and U16_MAX is
reserved as the "not set" value. skb_set_transport_header() silently
truncates an offset that does not fit.

sctp_v6_err() stores @offset, computed by icmpv6_notify() from the
extension headers of the packet embedded in an ICMPv6 error. Should
the value be truncated, sctp_err_lookup() would read the SCTP header
at the wrong place.

Since commit 3744b0964d52 ("ipv6: Implement limits on extension header
parsing") a single chain is bounded to IP6_MAX_EXT_HDRS_CNT headers
(24576 bytes), so this needs a large skb->data - skb->head on top of it,
for instance after several nested decapsulations, each of them possibly
pulling its own extension header chain, since @offset is relative to
the IPv6 header embedded in the ICMPv6 error. No reproducer was written,
this is hardening.

Use skb_set_transport_header_careful() and ignore the ICMPv6 error if
the offset can not be stored. Do this before changing the network
header, so that nothing has to be restored on failure.

Fixes: 1a37e412a022 ("net: Use 16bits for *_headers fields of struct skbuff")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 net/sctp/ipv6.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index 97390569c4bd..ad5c951efdf0 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -170,8 +170,11 @@ static int sctp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 	/* Fix up skb to look at the embedded net header. */
 	saveip	 = skb->network_header;
 	savesctp = skb->transport_header;
+	if (unlikely(!skb_set_transport_header_careful(skb, offset))) {
+		__ICMP6_INC_STATS(net, __in6_dev_get(skb->dev), ICMP6_MIB_INERRORS);
+		return -EINVAL;
+	}
 	skb_reset_network_header(skb);
-	skb_set_transport_header(skb, offset);
 	sk = sctp_err_lookup(net, AF_INET6, skb, sctp_hdr(skb), &asoc, &transport);
 	/* Put back, the original pointers. */
 	skb->network_header   = saveip;
-- 
2.53.0

[PATCH net 5/5] udp: use skb_set_transport_header_careful() in __udp6_lib_err_encap()

[Eric Dumazet]

skb->transport_header is a 16-bit offset from skb->head, and U16_MAX is
reserved as the "not set" value. skb_set_transport_header() silently
truncates an offset that does not fit.

__udp6_lib_err_encap() stores @offset, computed by icmpv6_notify() from
the extension headers of the packet embedded in an ICMPv6 error. Should
the value be truncated, the tunnel encap_err_lookup() and err_handler()
callbacks would read the UDP header at the wrong place.

Since commit 3744b0964d52 ("ipv6: Implement limits on extension header
parsing") a single chain is bounded to IP6_MAX_EXT_HDRS_CNT headers
(24576 bytes), so this needs a large skb->data - skb->head on top of it,
for instance after several nested decapsulations, each of them possibly
pulling its own extension header chain, since @offset is relative to
the IPv6 header embedded in the ICMPv6 error. No reproducer was written,
this is hardening.

Use skb_set_transport_header_careful() and return an error if the
offset can not be stored, in which case udpv6_err() ignores the
ICMPv6 error. Do this before changing the network header, so that
nothing has to be restored on failure.

Fixes: a36e185e8c85 ("udp: Handle ICMP errors for tunnels with same destination port on both endpoints")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 net/ipv6/udp.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 9dcefa162522..a6886a0432f9 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -651,12 +651,13 @@ static struct sock *__udp6_lib_err_encap(struct net *net,
 	network_offset = skb_network_offset(skb);
 	transport_offset = skb_transport_offset(skb);
 
+	/* Transport header needs to point to the UDP header */
+	if (unlikely(!skb_set_transport_header_careful(skb, offset)))
+		return ERR_PTR(-EINVAL);
+
 	/* Network header needs to point to the outer IPv6 header inside ICMP */
 	skb_reset_network_header(skb);
 
-	/* Transport header needs to point to the UDP header */
-	skb_set_transport_header(skb, offset);
-
 	if (sk) {
 		up = udp_sk(sk);
 
-- 
2.53.0