Skip to content

Commit

Permalink
vlan: consolidate VLAN parsing code and limit max parsing depth
Browse files Browse the repository at this point in the history
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 <toshiaki.makita1@gmail.com>
Reported-by: Daniel Borkmann <daniel@iogearbox.net>
Fixes: d7bf2eb ("sched: consistently handle layer3 header accesses in the presence of VLANs")
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
tohojo authored and davem330 committed Jul 7, 2020
1 parent da32871 commit 469aced
Showing 1 changed file with 22 additions and 35 deletions.
57 changes: 22 additions & 35 deletions include/linux/if_vlan.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 8 /* Max. number of nested VLAN tags parsed */

/*
* struct vlan_hdr - vlan header
* @h_vlan_TCI: priority and VLAN ID
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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
Expand All @@ -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));
Expand All @@ -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)
{
Expand Down

0 comments on commit 469aced

Please sign in to comment.