| Message ID | 1581697224-20041-1-git-send-email-jbaron@akamai.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v2,net] net: sched: correct flower port blocking | expand |
From: Jason Baron <jbaron@akamai.com> Date: Fri, 14 Feb 2020 11:20:24 -0500 > tc flower rules that are based on src or dst port blocking are sometimes > ineffective due to uninitialized stack data. __skb_flow_dissect() extracts > ports from the skb for tc flower to match against. However, the port > dissection is not done when when the FLOW_DIS_IS_FRAGMENT bit is set in > key_control->flags. All callers of __skb_flow_dissect(), zero-out the > key_control field except for fl_classify() as used by the flower > classifier. Thus, the FLOW_DIS_IS_FRAGMENT may be set on entry to > __skb_flow_dissect(), since key_control is allocated on the stack > and may not be initialized. > > Since key_basic and key_control are present for all flow keys, let's > make sure they are initialized. > > Fixes: 62230715fd24 ("flow_dissector: do not dissect l4 ports for fragments") > Co-developed-by: Eric Dumazet <edumazet@google.com> > Signed-off-by: Eric Dumazet <edumazet@google.com> > Acked-by: Cong Wang <xiyou.wangcong@gmail.com> > Signed-off-by: Jason Baron <jbaron@akamai.com> Applied and queued up for -stable.
From: David Miller <davem@davemloft.net> Date: Sun, 16 Feb 2020 19:08:12 -0800 (PST) > From: Jason Baron <jbaron@akamai.com> > Date: Fri, 14 Feb 2020 11:20:24 -0500 > >> tc flower rules that are based on src or dst port blocking are sometimes >> ineffective due to uninitialized stack data. __skb_flow_dissect() extracts >> ports from the skb for tc flower to match against. However, the port >> dissection is not done when when the FLOW_DIS_IS_FRAGMENT bit is set in >> key_control->flags. All callers of __skb_flow_dissect(), zero-out the >> key_control field except for fl_classify() as used by the flower >> classifier. Thus, the FLOW_DIS_IS_FRAGMENT may be set on entry to >> __skb_flow_dissect(), since key_control is allocated on the stack >> and may not be initialized. >> >> Since key_basic and key_control are present for all flow keys, let's >> make sure they are initialized. >> >> Fixes: 62230715fd24 ("flow_dissector: do not dissect l4 ports for fragments") >> Co-developed-by: Eric Dumazet <edumazet@google.com> >> Signed-off-by: Eric Dumazet <edumazet@google.com> >> Acked-by: Cong Wang <xiyou.wangcong@gmail.com> >> Signed-off-by: Jason Baron <jbaron@akamai.com> > > Applied and queued up for -stable. Actually this doesn't even compile: In file included from drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:33: ./include/net/flow_dissector.h: In function ‘flow_dissector_init_keys’: ./include/net/flow_dissector.h:355:2: error: implicit declaration of function ‘memset’ [-Werror=implicit-function-declaration] memset(key_control, 0, sizeof(*key_control)); ^~~~~~ ./include/net/flow_dissector.h:355:2: warning: incompatible implicit declaration of built-in function ‘memset’ ./include/net/flow_dissector.h:355:2: note: include ‘<string.h>’ or provide a declaration of ‘memset’ ./include/net/flow_dissector.h:9:1: +#include <string.h> ./include/net/flow_dissector.h:355:2: memset(key_control, 0, sizeof(*key_control)); ^~~~~~
diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h index d93017a..4bd4931 100644 --- a/include/net/flow_dissector.h +++ b/include/net/flow_dissector.h @@ -349,4 +349,12 @@ struct bpf_flow_dissector { void *data_end; }; +static inline void +flow_dissector_init_keys(struct flow_dissector_key_control *key_control, + struct flow_dissector_key_basic *key_basic) +{ + memset(key_control, 0, sizeof(*key_control)); + memset(key_basic, 0, sizeof(*key_basic)); +} + #endif diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c index f9c0d1e..b783254 100644 --- a/net/sched/cls_flower.c +++ b/net/sched/cls_flower.c @@ -305,6 +305,7 @@ static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp, struct cls_fl_filter *f; list_for_each_entry_rcu(mask, &head->masks, list) { + flow_dissector_init_keys(&skb_key.control, &skb_key.basic); fl_clear_masked_range(&skb_key, mask); skb_flow_dissect_meta(skb, &mask->dissector, &skb_key);
tc flower rules that are based on src or dst port blocking are sometimes ineffective due to uninitialized stack data. __skb_flow_dissect() extracts ports from the skb for tc flower to match against. However, the port dissection is not done when when the FLOW_DIS_IS_FRAGMENT bit is set in key_control->flags. All callers of __skb_flow_dissect(), zero-out the key_control field except for fl_classify() as used by the flower classifier. Thus, the FLOW_DIS_IS_FRAGMENT may be set on entry to __skb_flow_dissect(), since key_control is allocated on the stack and may not be initialized. Since key_basic and key_control are present for all flow keys, let's make sure they are initialized. Fixes: 62230715fd24 ("flow_dissector: do not dissect l4 ports for fragments") Co-developed-by: Eric Dumazet <edumazet@google.com> Signed-off-by: Eric Dumazet <edumazet@google.com> Acked-by: Cong Wang <xiyou.wangcong@gmail.com> Signed-off-by: Jason Baron <jbaron@akamai.com> --- v2: -move rename to flow_dissector_init_keys() amd move to flow_dissector.h (Cong Wang) include/net/flow_dissector.h | 8 ++++++++ net/sched/cls_flower.c | 1 + 2 files changed, 9 insertions(+)