On 2020/07/06 21:29, Toke Høiland-Jørgensen wrote:
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 <[email protected]>
Reported-by: Daniel Borkmann <[email protected]>
Fixes: d7bf2ebebc2b ("sched: consistently handle layer3 header accesses in the
presence of VLANs")
Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
---
...
@@ -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);
Some drivers which use vlan_get_protocol to get IP protocol for checksum
offload discards
packets when it cannot get the protocol.
I guess for such users this function should try to get protocol even if it is
not in skb header?
I'm not sure such a case can happen, but since you care about this, you know
real cases where
vlan tag can be in skb frags?
Toshiaki Makita
