| Message ID | 1581625699-24457-1-git-send-email-jbaron@akamai.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] net: sched: correct flower port blocking | expand |
On Thu, Feb 13, 2020 at 12:31 PM Jason Baron <jbaron@akamai.com> wrote: > > 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> > Signed-off-by: Jason Baron <jbaron@akamai.com> > --- > include/linux/skbuff.h | 8 ++++++++ > net/sched/cls_flower.c | 1 + > 2 files changed, 9 insertions(+) > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h > index ca8806b..f953bfa 100644 > --- a/include/linux/skbuff.h > +++ b/include/linux/skbuff.h > @@ -1288,6 +1288,14 @@ bool __skb_flow_dissect(const struct net *net, > void *data, __be16 proto, int nhoff, int hlen, > unsigned int flags); > > +static inline void > +skb_flow_dissect_init_key(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)); > +} I think this function has nothing to do with skb, therefore it fits better in include/net/flow_dissector.h? And remove skb prefix from its name too. Other than this: Acked-by: Cong Wang <xiyou.wangcong@gmail.com> Thanks.
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index ca8806b..f953bfa 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -1288,6 +1288,14 @@ bool __skb_flow_dissect(const struct net *net, void *data, __be16 proto, int nhoff, int hlen, unsigned int flags); +static inline void +skb_flow_dissect_init_key(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)); +} + static inline bool skb_flow_dissect(const struct sk_buff *skb, struct flow_dissector *flow_dissector, void *target_container, unsigned int flags) diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c index f9c0d1e..b0a534b 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) { + skb_flow_dissect_init_key(&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> Signed-off-by: Jason Baron <jbaron@akamai.com> --- include/linux/skbuff.h | 8 ++++++++ net/sched/cls_flower.c | 1 + 2 files changed, 9 insertions(+)