| Message ID | 20170913153237.26408-1-jiri@resnulli.us |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] net: sched: fix use-after-free in tcf_action_destroy and tcf_del_walker | expand |
From: Jiri Pirko <jiri@resnulli.us> Date: Wed, 13 Sep 2017 17:32:37 +0200 > From: Jiri Pirko <jiri@mellanox.com> > > Recent commit d7fb60b9cafb ("net_sched: get rid of tcfa_rcu") removed > freeing in call_rcu, which changed already existing hard-to-hit > race condition into 100% hit: > > [ 598.599825] BUG: unable to handle kernel NULL pointer dereference at 0000000000000030 > [ 598.607782] IP: tcf_action_destroy+0xc0/0x140 > > Or: > > [ 40.858924] BUG: unable to handle kernel NULL pointer dereference at 0000000000000030 > [ 40.862840] IP: tcf_generic_walker+0x534/0x820 > > Fix this by storing the ops and use them directly for module_put call. > > Fixes: a85a970af265 ("net_sched: move tc_action into tcf_common") > Signed-off-by: Jiri Pirko <jiri@mellanox.com> Applied, thanks Jiri.
Wed, Sep 13, 2017 at 06:34:28PM CEST, davem@davemloft.net wrote: >From: Jiri Pirko <jiri@resnulli.us> >Date: Wed, 13 Sep 2017 17:32:37 +0200 > >> From: Jiri Pirko <jiri@mellanox.com> >> >> Recent commit d7fb60b9cafb ("net_sched: get rid of tcfa_rcu") removed >> freeing in call_rcu, which changed already existing hard-to-hit >> race condition into 100% hit: >> >> [ 598.599825] BUG: unable to handle kernel NULL pointer dereference at 0000000000000030 >> [ 598.607782] IP: tcf_action_destroy+0xc0/0x140 >> >> Or: >> >> [ 40.858924] BUG: unable to handle kernel NULL pointer dereference at 0000000000000030 >> [ 40.862840] IP: tcf_generic_walker+0x534/0x820 >> >> Fix this by storing the ops and use them directly for module_put call. >> >> Fixes: a85a970af265 ("net_sched: move tc_action into tcf_common") >> Signed-off-by: Jiri Pirko <jiri@mellanox.com> > >Applied, thanks Jiri. Oh, I forgot to mention, this would be nice to push to stable.
From: Jiri Pirko <jiri@resnulli.us> Date: Wed, 13 Sep 2017 22:50:06 +0200 > Wed, Sep 13, 2017 at 06:34:28PM CEST, davem@davemloft.net wrote: >>From: Jiri Pirko <jiri@resnulli.us> >>Date: Wed, 13 Sep 2017 17:32:37 +0200 >> >>> From: Jiri Pirko <jiri@mellanox.com> >>> >>> Recent commit d7fb60b9cafb ("net_sched: get rid of tcfa_rcu") removed >>> freeing in call_rcu, which changed already existing hard-to-hit >>> race condition into 100% hit: >>> >>> [ 598.599825] BUG: unable to handle kernel NULL pointer dereference at 0000000000000030 >>> [ 598.607782] IP: tcf_action_destroy+0xc0/0x140 >>> >>> Or: >>> >>> [ 40.858924] BUG: unable to handle kernel NULL pointer dereference at 0000000000000030 >>> [ 40.862840] IP: tcf_generic_walker+0x534/0x820 >>> >>> Fix this by storing the ops and use them directly for module_put call. >>> >>> Fixes: a85a970af265 ("net_sched: move tc_action into tcf_common") >>> Signed-off-by: Jiri Pirko <jiri@mellanox.com> >> >>Applied, thanks Jiri. > > Oh, I forgot to mention, this would be nice to push to stable. Ok, queued up.
diff --git a/net/sched/act_api.c b/net/sched/act_api.c index fcd7dc7..da6fa82 100644 --- a/net/sched/act_api.c +++ b/net/sched/act_api.c @@ -180,7 +180,7 @@ static int tcf_del_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb, idr_for_each_entry_ext(idr, p, id) { ret = __tcf_idr_release(p, false, true); if (ret == ACT_P_DELETED) { - module_put(p->ops->owner); + module_put(ops->owner); n_i++; } else if (ret < 0) { goto nla_put_failure; @@ -514,13 +514,15 @@ EXPORT_SYMBOL(tcf_action_exec); int tcf_action_destroy(struct list_head *actions, int bind) { + const struct tc_action_ops *ops; struct tc_action *a, *tmp; int ret = 0; list_for_each_entry_safe(a, tmp, actions, list) { + ops = a->ops; ret = __tcf_idr_release(a, bind, true); if (ret == ACT_P_DELETED) - module_put(a->ops->owner); + module_put(ops->owner); else if (ret < 0) return ret; }