On 7/6/20 2:29 PM, 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]>
---
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 */
+
Any insight on limits of nesting wrt QinQ, maybe from spec side? Why not 8 as
max, for
example (I'd probably even consider a depth like this as utterly broken setup
..)?
Thanks,
Daniel
_______________________________________________
Cake mailing list
[email protected]
https://lists.bufferbloat.net/listinfo/cake
