Skip to content

Commit 469aced

Browse files
tohojodavem330
authored andcommitted
vlan: consolidate VLAN parsing code and limit max parsing depth
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 a max depth of 8 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. v2: - Use limit of 8 tags instead of 32 (matching XMIT_RECURSION_LIMIT) Reported-by: Toshiaki Makita <[email protected]> Reported-by: Daniel Borkmann <[email protected]> Fixes: d7bf2eb ("sched: consistently handle layer3 header accesses in the presence of VLANs") Signed-off-by: Toke Høiland-Jørgensen <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent da32871 commit 469aced

File tree

1 file changed

+22
-35
lines changed

1 file changed

+22
-35
lines changed

include/linux/if_vlan.h

Lines changed: 22 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
#define VLAN_ETH_DATA_LEN 1500 /* Max. octets in payload */
2626
#define VLAN_ETH_FRAME_LEN 1518 /* Max. octets in frame sans FCS */
2727

28+
#define VLAN_MAX_DEPTH 8 /* Max. number of nested VLAN tags parsed */
29+
2830
/*
2931
* struct vlan_hdr - vlan header
3032
* @h_vlan_TCI: priority and VLAN ID
@@ -308,34 +310,6 @@ static inline bool eth_type_vlan(__be16 ethertype)
308310
}
309311
}
310312

311-
/* A getter for the SKB protocol field which will handle VLAN tags consistently
312-
* whether VLAN acceleration is enabled or not.
313-
*/
314-
static inline __be16 skb_protocol(const struct sk_buff *skb, bool skip_vlan)
315-
{
316-
unsigned int offset = skb_mac_offset(skb) + sizeof(struct ethhdr);
317-
__be16 proto = skb->protocol;
318-
319-
if (!skip_vlan)
320-
/* VLAN acceleration strips the VLAN header from the skb and
321-
* moves it to skb->vlan_proto
322-
*/
323-
return skb_vlan_tag_present(skb) ? skb->vlan_proto : proto;
324-
325-
while (eth_type_vlan(proto)) {
326-
struct vlan_hdr vhdr, *vh;
327-
328-
vh = skb_header_pointer(skb, offset, sizeof(vhdr), &vhdr);
329-
if (!vh)
330-
break;
331-
332-
proto = vh->h_vlan_encapsulated_proto;
333-
offset += sizeof(vhdr);
334-
}
335-
336-
return proto;
337-
}
338-
339313
static inline bool vlan_hw_offload_capable(netdev_features_t features,
340314
__be16 proto)
341315
{
@@ -605,10 +579,10 @@ static inline int vlan_get_tag(const struct sk_buff *skb, u16 *vlan_tci)
605579
* Returns the EtherType of the packet, regardless of whether it is
606580
* vlan encapsulated (normal or hardware accelerated) or not.
607581
*/
608-
static inline __be16 __vlan_get_protocol(struct sk_buff *skb, __be16 type,
582+
static inline __be16 __vlan_get_protocol(const struct sk_buff *skb, __be16 type,
609583
int *depth)
610584
{
611-
unsigned int vlan_depth = skb->mac_len;
585+
unsigned int vlan_depth = skb->mac_len, parse_depth = VLAN_MAX_DEPTH;
612586

613587
/* if type is 802.1Q/AD then the header should already be
614588
* 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,
623597
vlan_depth = ETH_HLEN;
624598
}
625599
do {
626-
struct vlan_hdr *vh;
600+
struct vlan_hdr vhdr, *vh;
627601

628-
if (unlikely(!pskb_may_pull(skb,
629-
vlan_depth + VLAN_HLEN)))
602+
vh = skb_header_pointer(skb, vlan_depth, sizeof(vhdr), &vhdr);
603+
if (unlikely(!vh || !--parse_depth))
630604
return 0;
631605

632-
vh = (struct vlan_hdr *)(skb->data + vlan_depth);
633606
type = vh->h_vlan_encapsulated_proto;
634607
vlan_depth += VLAN_HLEN;
635608
} while (eth_type_vlan(type));
@@ -648,11 +621,25 @@ static inline __be16 __vlan_get_protocol(struct sk_buff *skb, __be16 type,
648621
* Returns the EtherType of the packet, regardless of whether it is
649622
* vlan encapsulated (normal or hardware accelerated) or not.
650623
*/
651-
static inline __be16 vlan_get_protocol(struct sk_buff *skb)
624+
static inline __be16 vlan_get_protocol(const struct sk_buff *skb)
652625
{
653626
return __vlan_get_protocol(skb, skb->protocol, NULL);
654627
}
655628

629+
/* A getter for the SKB protocol field which will handle VLAN tags consistently
630+
* whether VLAN acceleration is enabled or not.
631+
*/
632+
static inline __be16 skb_protocol(const struct sk_buff *skb, bool skip_vlan)
633+
{
634+
if (!skip_vlan)
635+
/* VLAN acceleration strips the VLAN header from the skb and
636+
* moves it to skb->vlan_proto
637+
*/
638+
return skb_vlan_tag_present(skb) ? skb->vlan_proto : skb->protocol;
639+
640+
return vlan_get_protocol(skb);
641+
}
642+
656643
static inline void vlan_set_encap_proto(struct sk_buff *skb,
657644
struct vlan_hdr *vhdr)
658645
{

0 commit comments

Comments
 (0)