All of lore.kernel.org
 help / color / mirror / Atom feed
From: Balazs Nemeth <bnemeth@redhat.com>
To: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, mst@redhat.com,
	jasowang@redhat.com, davem@davemloft.net, willemb@google.com,
	virtualization@lists.linux-foundation.org, bnemeth@redhat.com
Subject: [PATCH] net: check if protocol extracted by virtio_net_hdr_set_proto is correct
Date: Thu, 18 Feb 2021 15:57:54 +0100	[thread overview]
Message-ID: <5e910d11a14da17c41317417fc41d3a9d472c6e7.1613659844.git.bnemeth@redhat.com> (raw)

For gso packets, virtio_net_hdr_set_proto sets the protocol (if it isn't
set) based on the type in the virtio net hdr, but the skb could contain
anything since it could come from packet_snd through a raw socket. If
there is a mismatch between what virtio_net_hdr_set_proto sets and
the actual protocol, then the skb could be handled incorrectly later
on by gso.

The network header of gso packets starts at 14 bytes, but a specially
crafted packet could fool the call to skb_flow_dissect_flow_keys_basic
as the network header offset in the skb could be incorrect.
Consequently, EINVAL is not returned.

There are even packets that can cause an infinite loop. For example, a
packet with ethernet type ETH_P_MPLS_UC (which is unnoticed by
virtio_net_hdr_to_skb) that is sent to a geneve interface will be
handled by geneve_build_skb. In turn, it calls
udp_tunnel_handle_offloads which then calls skb_reset_inner_headers.
After that, the packet gets passed to mpls_gso_segment. That function
calculates the mpls header length by taking the difference between
network_header and inner_network_header. Since the two are equal
(due to the earlier call to skb_reset_inner_headers), it will calculate
a header of length 0, and it will not pull any headers. Then, it will
call skb_mac_gso_segment which will again call mpls_gso_segment, etc...
This leads to the infinite loop.

For that reason, address the root cause of the issue: don't blindly
trust the information provided by the virtio net header. Instead,
check if the protocol in the packet actually matches the protocol set by
virtio_net_hdr_set_proto.

Fixes: 9274124f023b ("net: stricter validation of untrusted gso packets")
Signed-off-by: Balazs Nemeth <bnemeth@redhat.com>
---
 include/linux/virtio_net.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index e8a924eeea3d..cf2c53563f22 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -79,8 +79,13 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
 		if (gso_type && skb->network_header) {
 			struct flow_keys_basic keys;
 
-			if (!skb->protocol)
+			if (!skb->protocol) {
+				const struct ethhdr *eth = skb_eth_hdr(skb);
+
 				virtio_net_hdr_set_proto(skb, hdr);
+				if (skb->protocol != eth->h_proto)
+					return -EINVAL;
+			}
 retry:
 			if (!skb_flow_dissect_flow_keys_basic(NULL, skb, &keys,
 							      NULL, 0, 0, 0,
-- 
2.29.2


             reply	other threads:[~2021-02-18 17:45 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-18 14:57 Balazs Nemeth [this message]
2021-02-18 15:50 ` [PATCH] net: check if protocol extracted by virtio_net_hdr_set_proto is correct Willem de Bruijn
2021-02-18 15:50   ` Willem de Bruijn
2021-02-19  8:51   ` Jason Wang
2021-02-19  8:51     ` Jason Wang
2021-02-19 14:55     ` Willem de Bruijn
2021-02-19 14:55       ` Willem de Bruijn
2021-02-22  3:39       ` Jason Wang
2021-02-22  3:39         ` Jason Wang
2021-02-23 13:42         ` Balazs Nemeth
2021-02-23 14:22           ` Willem de Bruijn
2021-02-23 14:22             ` Willem de Bruijn

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=5e910d11a14da17c41317417fc41d3a9d472c6e7.1613659844.git.bnemeth@redhat.com \
    --to=bnemeth@redhat.com \
    --cc=davem@davemloft.net \
    --cc=jasowang@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=willemb@google.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.