queue-4.20/net-sched-flower-insert-new-filter-to-idr-after-setting-its-mask.patch

   1 From foo@baz Thu Mar 14 23:20:15 PDT 2019
   2 From: Vlad Buslov <vladbu@mellanox.com>
   3 Date: Wed, 6 Mar 2019 16:22:12 +0200
   4 Subject: net: sched: flower: insert new filter to idr after setting its mask
   5
   6 From: Vlad Buslov <vladbu@mellanox.com>
   7
   8 [ Upstream commit ecb3dea400d3beaf611ce76ac7a51d4230492cf2 ]
   9
  10 When adding new filter to flower classifier, fl_change() inserts it to
  11 handle_idr before initializing filter extensions and assigning it a mask.
  12 Normally this ordering doesn't matter because all flower classifier ops
  13 callbacks assume rtnl lock protection. However, when filter has an action
  14 that doesn't have its kernel module loaded, rtnl lock is released before
  15 call to request_module(). During this time the filter can be accessed bu
  16 concurrent task before its initialization is completed, which can lead to a
  17 crash.
  18
  19 Example case of NULL pointer dereference in concurrent dump:
  20
  21 Task 1                           Task 2
  22
  23 tc_new_tfilter()
  24  fl_change()
  25   idr_alloc_u32(fnew)
  26   fl_set_parms()
  27    tcf_exts_validate()
  28     tcf_action_init()
  29      tcf_action_init_1()
  30       rtnl_unlock()
  31       request_module()
  32       ...                        rtnl_lock()
  33                                  tc_dump_tfilter()
  34                                   tcf_chain_dump()
  35                                    fl_walk()
  36                                     idr_get_next_ul()
  37                                     tcf_node_dump()
  38                                      tcf_fill_node()
  39                                       fl_dump()
  40                                        mask = &f->mask->key; <- NULL ptr
  41       rtnl_lock()
  42
  43 Extension initialization and mask assignment don't depend on fnew->handle
  44 that is allocated by idr_alloc_u32(). Move idr allocation code after action
  45 creation and mask assignment in fl_change() to prevent concurrent access
  46 to not fully initialized filter when rtnl lock is released to load action
  47 module.
  48
  49 Fixes: 01683a146999 ("net: sched: refactor flower walk to iterate over idr")
  50 Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
  51 Reviewed-by: Roi Dayan <roid@mellanox.com>
  52 Signed-off-by: David S. Miller <davem@davemloft.net>
  53 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  54 ---
  55  net/sched/cls_flower.c |   43 ++++++++++++++++++++++---------------------
  56  1 file changed, 22 insertions(+), 21 deletions(-)
  57
  58 --- a/net/sched/cls_flower.c
  59 +++ b/net/sched/cls_flower.c
  60 @@ -1213,46 +1213,46 @@ static int fl_change(struct net *net, st
  61         if (err < 0)
  62                 goto errout;
  63
  64 -       if (!handle) {
  65 -               handle = 1;
  66 -               err = idr_alloc_u32(&head->handle_idr, fnew, &handle,
  67 -                                   INT_MAX, GFP_KERNEL);
  68 -       } else if (!fold) {
  69 -               /* user specifies a handle and it doesn't exist */
  70 -               err = idr_alloc_u32(&head->handle_idr, fnew, &handle,
  71 -                                   handle, GFP_KERNEL);
  72 -       }
  73 -       if (err)
  74 -               goto errout;
  75 -       fnew->handle = handle;
  76 -
  77         if (tb[TCA_FLOWER_FLAGS]) {
  78                 fnew->flags = nla_get_u32(tb[TCA_FLOWER_FLAGS]);
  79
  80                 if (!tc_flags_valid(fnew->flags)) {
  81                         err = -EINVAL;
  82 -                       goto errout_idr;
  83 +                       goto errout;
  84                 }
  85         }
  86
  87         err = fl_set_parms(net, tp, fnew, mask, base, tb, tca[TCA_RATE], ovr,
  88                            tp->chain->tmplt_priv, extack);
  89         if (err)
  90 -               goto errout_idr;
  91 +               goto errout;
  92
  93         err = fl_check_assign_mask(head, fnew, fold, mask);
  94         if (err)
  95 -               goto errout_idr;
  96 +               goto errout;
  97 +
  98 +       if (!handle) {
  99 +               handle = 1;
 100 +               err = idr_alloc_u32(&head->handle_idr, fnew, &handle,
 101 +                                   INT_MAX, GFP_KERNEL);
 102 +       } else if (!fold) {
 103 +               /* user specifies a handle and it doesn't exist */
 104 +               err = idr_alloc_u32(&head->handle_idr, fnew, &handle,
 105 +                                   handle, GFP_KERNEL);
 106 +       }
 107 +       if (err)
 108 +               goto errout_mask;
 109 +       fnew->handle = handle;
 110
 111         if (!fold && fl_lookup(fnew->mask, &fnew->mkey)) {
 112                 err = -EEXIST;
 113 -               goto errout_mask;
 114 +               goto errout_idr;
 115         }
 116
 117         err = rhashtable_insert_fast(&fnew->mask->ht, &fnew->ht_node,
 118                                      fnew->mask->filter_ht_params);
 119         if (err)
 120 -               goto errout_mask;
 121 +               goto errout_idr;
 122
 123         if (!tc_skip_hw(fnew->flags)) {
 124                 err = fl_hw_replace_filter(tp, fnew, extack);
 125 @@ -1291,12 +1291,13 @@ errout_mask_ht:
 126         rhashtable_remove_fast(&fnew->mask->ht, &fnew->ht_node,
 127                                fnew->mask->filter_ht_params);
 128
 129 -errout_mask:
 130 -       fl_mask_put(head, fnew->mask, false);
 131 -
 132  errout_idr:
 133         if (!fold)
 134                 idr_remove(&head->handle_idr, fnew->handle);
 135 +
 136 +errout_mask:
 137 +       fl_mask_put(head, fnew->mask, false);
 138 +
 139  errout:
 140         tcf_exts_destroy(&fnew->exts);
 141         kfree(fnew);