All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guillaume Nault <gnault@redhat.com>
To: David Miller <davem@davemloft.net>, Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org,
	Martin Varghese <martin.varghese@nokia.com>,
	Davide Caratti <dcaratti@redhat.com>
Subject: [PATCH net] net/core: check length before updating Ethertype in skb_mpls_{push,pop}
Date: Fri, 2 Oct 2020 21:53:08 +0200	[thread overview]
Message-ID: <71ec98d51cc4aab7615061336fb1498ad16cda30.1601667845.git.gnault@redhat.com> (raw)

Openvswitch allows to drop a packet's Ethernet header, therefore
skb_mpls_push() and skb_mpls_pop() might be called with ethernet=true
and mac_len=0. In that case the pointer passed to skb_mod_eth_type()
doesn't point to an Ethernet header and the new Ethertype is written at
unexpected locations.

Fix this by verifying that mac_len is big enough to contain an Ethernet
header.

Fixes: fa4e0f8855fc ("net/sched: fix corrupted L2 header with MPLS 'push' and 'pop' actions")
Signed-off-by: Guillaume Nault <gnault@redhat.com>
---
Notes:
  - Found by code inspection.
  - Using commit fa4e0f8855fc for the Fixes tag because mac_len is
    needed for the test. The problem probably exists since openvswitch
    can pop the Ethernet header though.

 net/core/skbuff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 6faf73d6a0f7..2b48cb0cc684 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -5622,7 +5622,7 @@ int skb_mpls_push(struct sk_buff *skb, __be32 mpls_lse, __be16 mpls_proto,
 	lse->label_stack_entry = mpls_lse;
 	skb_postpush_rcsum(skb, lse, MPLS_HLEN);
 
-	if (ethernet)
+	if (ethernet && mac_len >= ETH_HLEN)
 		skb_mod_eth_type(skb, eth_hdr(skb), mpls_proto);
 	skb->protocol = mpls_proto;
 
@@ -5662,7 +5662,7 @@ int skb_mpls_pop(struct sk_buff *skb, __be16 next_proto, int mac_len,
 	skb_reset_mac_header(skb);
 	skb_set_network_header(skb, mac_len);
 
-	if (ethernet) {
+	if (ethernet && mac_len >= ETH_HLEN) {
 		struct ethhdr *hdr;
 
 		/* use mpls_hdr() to get ethertype to account for VLANs. */
-- 
2.21.3


             reply	other threads:[~2020-10-02 19:53 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-02 19:53 Guillaume Nault [this message]
2020-10-04 14:00 ` [PATCH net] net/core: check length before updating Ethertype in skb_mpls_{push,pop} Davide Caratti
2020-10-04 14:50 ` Varghese, Martin (Nokia - IN/Bangalore)
2020-10-04 21:29   ` David Miller
2020-10-04 22:10 ` David Miller

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=71ec98d51cc4aab7615061336fb1498ad16cda30.1601667845.git.gnault@redhat.com \
    --to=gnault@redhat.com \
    --cc=davem@davemloft.net \
    --cc=dcaratti@redhat.com \
    --cc=kuba@kernel.org \
    --cc=martin.varghese@nokia.com \
    --cc=netdev@vger.kernel.org \
    /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.