Commit cd0c4e70 authored Jan 11, 2019 by Cong Wang Committed by David S. Miller Jan 16, 2019

net_sched: refetch skb protocol for each filter



Martin reported a set of filters don't work after changing
from reclassify to continue. Looking into the code, it
looks like skb protocol is not always fetched for each
iteration of the filters. But, as demonstrated by Martin,
TC actions could modify skb->protocol, for example act_vlan,
this means we have to refetch skb protocol in each iteration,
rather than using the one we fetch in the beginning of the loop.

This bug is _not_ introduced by commit 3b3ae880
("net: sched: consolidate tc_classify{,_compat}"), technically,
if act_vlan is the only action that modifies skb protocol, then
it is commit c7e2b968 ("sched: introduce vlan action") which
introduced this bug.

Reported-by: Martin Olsson <martin.olsson+netdev@sentorsecurity.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

parent f2f98c1d

net/sched/cls_api.c

+1 −2

Original line number	Original line	Diff line number	Diff line
	@@ -1277,7 +1277,6 @@ EXPORT_SYMBOL(tcf_block_cb_unregister);
	int tcf_classify(struct sk_buff skb, const struct tcf_proto tp,		int tcf_classify(struct sk_buff skb, const struct tcf_proto tp,
	struct tcf_result *res, bool compat_mode)		struct tcf_result *res, bool compat_mode)
	{		{
	__be16 protocol = tc_skb_protocol(skb);
	#ifdef CONFIG_NET_CLS_ACT		#ifdef CONFIG_NET_CLS_ACT
	const int max_reclassify_loop = 4;		const int max_reclassify_loop = 4;
	const struct tcf_proto *orig_tp = tp;		const struct tcf_proto *orig_tp = tp;
	@@ -1287,6 +1286,7 @@ int tcf_classify(struct sk_buff skb, const struct tcf_proto tp,
	reclassify:		reclassify:
	#endif		#endif
	for (; tp; tp = rcu_dereference_bh(tp->next)) {		for (; tp; tp = rcu_dereference_bh(tp->next)) {
			__be16 protocol = tc_skb_protocol(skb);
	int err;		int err;

	if (tp->protocol != protocol &&		if (tp->protocol != protocol &&
	@@ -1319,7 +1319,6 @@ reset:
	}		}

	tp = first_tp;		tp = first_tp;
	protocol = tc_skb_protocol(skb);
	goto reclassify;		goto reclassify;
	#endif		#endif
	}		}

Admin message