From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com [205.139.110.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id E1BC13B2A4 for ; Mon, 6 Jul 2020 08:30:39 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1594038639; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=wXWAdqNpNpJ8PzmXu+LnUcnHfb9Qo0xXxjNDy2nWeNs=; b=P2CVz14e9TDBHG5lg+6Xd/eqWu66tVVi0EgFPORiGatISnXyAglDfV6YamzWeITONsCYzO t3spEIiywC/KBVIXgFRBq+Tievg6wokWXESVPrMYzaDVRkRn6yYowsQSsubf0uGJvvB4bp w8HP1lam7r6QE7A+oSEHclKxtU0t8b8= Received: from mail-io1-f70.google.com (mail-io1-f70.google.com [209.85.166.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-198-P55XmcZ1NsKQVM5-MW76pA-1; Mon, 06 Jul 2020 08:30:37 -0400 X-MC-Unique: P55XmcZ1NsKQVM5-MW76pA-1 Received: by mail-io1-f70.google.com with SMTP id k12so13437056iom.19 for ; Mon, 06 Jul 2020 05:30:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=wXWAdqNpNpJ8PzmXu+LnUcnHfb9Qo0xXxjNDy2nWeNs=; b=KUxD5uC7AM8xCWrwf49aATurWEvdzbs54+AzXbPIZN2UEb8RWGbE5BfTd1xyQTAHns sgscO4Q5TWjXZsA9oS67F9EyBkvXyI5FQQARNu8d3ebRq5X1ShWehaBGjQvVyiokUip2 0oiQfSAIvJ1rLsvoTVnCY/fVPmu68YcaNV45jFi1iZk9J5GAGSCPHPgGr5osuAI0L0pN POYBkcm/tgHB/NaHzW//Hb6c87Qg6ZwT2R2LxmOPRq92vooo3Tbm9+M0lWfVZ5ghhKJn uwReuR3HMJStfL6dG/bmnUajBegBHvx/zTc+VrDlWvqH3k99uELxnhMn0a9IKMUh5Fn6 wcfQ== X-Gm-Message-State: AOAM533/AfICwCXsY+cy7N/6N4i5dEGPcHZOuDjBgkzXHkTBQlupSM0D vyWptrsYVFmSBot1XGpJHBnYyuhPaBEDRaQQFgBMdoKNGRxj0xKhcnrT6/Dua3iOKx/76rKYrYI I9GOWHPYTcOekmtN4V8UQLA== X-Received: by 2002:a92:d9c4:: with SMTP id n4mr13321189ilq.280.1594038637288; Mon, 06 Jul 2020 05:30:37 -0700 (PDT) X-Google-Smtp-Source: ABdhPJz3xF55IVY4TMVryG+hd/owtJ6QxxvTlfa8bAab9hAsQJBUZzelZQCb4KQXHAJv57LXdauSGg== X-Received: by 2002:a92:d9c4:: with SMTP id n4mr13321160ilq.280.1594038636982; Mon, 06 Jul 2020 05:30:36 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([45.145.92.2]) by smtp.gmail.com with ESMTPSA id m16sm9955840ili.26.2020.07.06.05.30.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 Jul 2020 05:30:36 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id 1C70C1804EA; Mon, 6 Jul 2020 14:30:34 +0200 (CEST) From: =?UTF-8?q?Toke=20H=C3=B8iland-J=C3=B8rgensen?= To: davem@davemloft.net Cc: =?UTF-8?q?Toke=20H=C3=B8iland-J=C3=B8rgensen?= , netdev@vger.kernel.org, cake@lists.bufferbloat.net, Davide Caratti , Jiri Pirko , Jamal Hadi Salim , Cong Wang , Toshiaki Makita , Daniel Borkmann Date: Mon, 6 Jul 2020 14:29:51 +0200 Message-Id: <20200706122951.48142-1-toke@redhat.com> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=toke@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Mailman-Approved-At: Mon, 06 Jul 2020 08:31:42 -0400 Subject: [Cake] [PATCH net] vlan: consolidate VLAN parsing code and limit max parsing depth X-BeenThere: cake@lists.bufferbloat.net X-Mailman-Version: 2.1.20 Precedence: list List-Id: Cake - FQ_codel the next generation List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Jul 2020 12:30:40 -0000 Toshiaki pointed out that we now have two very similar functions to extract the L3 protocol number in the presence of VLAN tags. And Daniel pointed out that the unbounded parsing loop makes it possible for maliciously crafted packets to loop through potentially hundreds of tags. Fix both of these issues by consolidating the two parsing functions and limiting the VLAN tag parsing to an arbitrarily-chosen, but hopefully conservative, max depth of 32 tags. As part of this, switch over __vlan_get_protocol() to use skb_header_pointer() instead of pskb_may_pull(), to avoid the possible side effects of the latter and keep the skb pointer 'const' through all the parsing functions. Reported-by: Toshiaki Makita Reported-by: Daniel Borkmann Fixes: d7bf2ebebc2b ("sched: consistently handle layer3 header accesses in the presence of VLANs") Signed-off-by: Toke Høiland-Jørgensen --- include/linux/if_vlan.h | 57 ++++++++++++++++------------------------- 1 file changed, 22 insertions(+), 35 deletions(-) diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h index 427a5b8597c2..855d16192e6a 100644 --- a/include/linux/if_vlan.h +++ b/include/linux/if_vlan.h @@ -25,6 +25,8 @@ #define VLAN_ETH_DATA_LEN 1500 /* Max. octets in payload */ #define VLAN_ETH_FRAME_LEN 1518 /* Max. octets in frame sans FCS */ +#define VLAN_MAX_DEPTH 32 /* Max. number of nested VLAN tags parsed */ + /* * struct vlan_hdr - vlan header * @h_vlan_TCI: priority and VLAN ID @@ -308,34 +310,6 @@ static inline bool eth_type_vlan(__be16 ethertype) } } -/* A getter for the SKB protocol field which will handle VLAN tags consistently - * whether VLAN acceleration is enabled or not. - */ -static inline __be16 skb_protocol(const struct sk_buff *skb, bool skip_vlan) -{ - unsigned int offset = skb_mac_offset(skb) + sizeof(struct ethhdr); - __be16 proto = skb->protocol; - - if (!skip_vlan) - /* VLAN acceleration strips the VLAN header from the skb and - * moves it to skb->vlan_proto - */ - return skb_vlan_tag_present(skb) ? skb->vlan_proto : proto; - - while (eth_type_vlan(proto)) { - struct vlan_hdr vhdr, *vh; - - vh = skb_header_pointer(skb, offset, sizeof(vhdr), &vhdr); - if (!vh) - break; - - proto = vh->h_vlan_encapsulated_proto; - offset += sizeof(vhdr); - } - - return proto; -} - static inline bool vlan_hw_offload_capable(netdev_features_t features, __be16 proto) { @@ -605,10 +579,10 @@ static inline int vlan_get_tag(const struct sk_buff *skb, u16 *vlan_tci) * Returns the EtherType of the packet, regardless of whether it is * vlan encapsulated (normal or hardware accelerated) or not. */ -static inline __be16 __vlan_get_protocol(struct sk_buff *skb, __be16 type, +static inline __be16 __vlan_get_protocol(const struct sk_buff *skb, __be16 type, int *depth) { - unsigned int vlan_depth = skb->mac_len; + unsigned int vlan_depth = skb->mac_len, parse_depth = VLAN_MAX_DEPTH; /* if type is 802.1Q/AD then the header should already be * present at mac_len - VLAN_HLEN (if mac_len > 0), or at @@ -623,13 +597,12 @@ static inline __be16 __vlan_get_protocol(struct sk_buff *skb, __be16 type, vlan_depth = ETH_HLEN; } do { - struct vlan_hdr *vh; + struct vlan_hdr vhdr, *vh; - if (unlikely(!pskb_may_pull(skb, - vlan_depth + VLAN_HLEN))) + vh = skb_header_pointer(skb, vlan_depth, sizeof(vhdr), &vhdr); + if (unlikely(!vh || !--parse_depth)) return 0; - vh = (struct vlan_hdr *)(skb->data + vlan_depth); type = vh->h_vlan_encapsulated_proto; vlan_depth += VLAN_HLEN; } while (eth_type_vlan(type)); @@ -648,11 +621,25 @@ static inline __be16 __vlan_get_protocol(struct sk_buff *skb, __be16 type, * Returns the EtherType of the packet, regardless of whether it is * vlan encapsulated (normal or hardware accelerated) or not. */ -static inline __be16 vlan_get_protocol(struct sk_buff *skb) +static inline __be16 vlan_get_protocol(const struct sk_buff *skb) { return __vlan_get_protocol(skb, skb->protocol, NULL); } +/* A getter for the SKB protocol field which will handle VLAN tags consistently + * whether VLAN acceleration is enabled or not. + */ +static inline __be16 skb_protocol(const struct sk_buff *skb, bool skip_vlan) +{ + if (!skip_vlan) + /* VLAN acceleration strips the VLAN header from the skb and + * moves it to skb->vlan_proto + */ + return skb_vlan_tag_present(skb) ? skb->vlan_proto : skb->protocol; + + return vlan_get_protocol(skb); +} + static inline void vlan_set_encap_proto(struct sk_buff *skb, struct vlan_hdr *vhdr) { -- 2.27.0