Make gact->param RCU-protected and reclaim old params via call_rcu(). This follows the pattern used by other actions: act_pedit swaps params with rcu_replace_pointer() and defers free via call_rcu() (commit 52cf89f78c01bf), act_connmark uses rcu_replace_pointer() under tcf_lock (commit 288864effe3388), and act_tunnel_key does the same under lockdep (commit 445d3749315f34). Dump readers in act_ct and act_pedit already use rcu_read_lock() + rcu_dereference() (commits 554e66bad84ce4 and 9d096746572616), so act_gate must keep old params alive past updates as well. Fixes: a51c328df310 ("net: qos: introduce a gate control flow action") Signed-off-by: Paul Moses Cc: stable@vger.kernel.org --- include/net/tc_act/tc_gate.h | 31 ++++++++++++++----- net/sched/act_gate.c | 59 +++++++++++++++++++++++++++--------- 2 files changed, 69 insertions(+), 21 deletions(-) diff --git a/include/net/tc_act/tc_gate.h b/include/net/tc_act/tc_gate.h index c1a67149c6b62..05968b3822392 100644 --- a/include/net/tc_act/tc_gate.h +++ b/include/net/tc_act/tc_gate.h @@ -32,6 +32,7 @@ struct tcf_gate_params { s32 tcfg_clockid; size_t num_entries; struct list_head entries; + struct rcu_head rcu; }; #define GATE_ACT_GATE_OPEN BIT(0) @@ -39,7 +40,7 @@ struct tcf_gate_params { struct tcf_gate { struct tc_action common; - struct tcf_gate_params param; + struct tcf_gate_params __rcu *param; u8 current_gate_status; ktime_t current_close_time; u32 current_entry_octets; @@ -54,8 +55,11 @@ struct tcf_gate { static inline s32 tcf_gate_prio(const struct tc_action *a) { s32 tcfg_prio; + struct tcf_gate_params *p; - tcfg_prio = to_gate(a)->param.tcfg_priority; + p = rcu_dereference_protected(to_gate(a)->param, + lockdep_rtnl_is_held()); + tcfg_prio = p->tcfg_priority; return tcfg_prio; } @@ -63,8 +67,11 @@ static inline s32 tcf_gate_prio(const struct tc_action *a) static inline u64 tcf_gate_basetime(const struct tc_action *a) { u64 tcfg_basetime; + struct tcf_gate_params *p; - tcfg_basetime = to_gate(a)->param.tcfg_basetime; + p = rcu_dereference_protected(to_gate(a)->param, + lockdep_rtnl_is_held()); + tcfg_basetime = p->tcfg_basetime; return tcfg_basetime; } @@ -72,8 +79,11 @@ static inline u64 tcf_gate_basetime(const struct tc_action *a) static inline u64 tcf_gate_cycletime(const struct tc_action *a) { u64 tcfg_cycletime; + struct tcf_gate_params *p; - tcfg_cycletime = to_gate(a)->param.tcfg_cycletime; + p = rcu_dereference_protected(to_gate(a)->param, + lockdep_rtnl_is_held()); + tcfg_cycletime = p->tcfg_cycletime; return tcfg_cycletime; } @@ -81,8 +91,11 @@ static inline u64 tcf_gate_cycletime(const struct tc_action *a) static inline u64 tcf_gate_cycletimeext(const struct tc_action *a) { u64 tcfg_cycletimeext; + struct tcf_gate_params *p; - tcfg_cycletimeext = to_gate(a)->param.tcfg_cycletime_ext; + p = rcu_dereference_protected(to_gate(a)->param, + lockdep_rtnl_is_held()); + tcfg_cycletimeext = p->tcfg_cycletime_ext; return tcfg_cycletimeext; } @@ -90,8 +103,11 @@ static inline u64 tcf_gate_cycletimeext(const struct tc_action *a) static inline u32 tcf_gate_num_entries(const struct tc_action *a) { u32 num_entries; + struct tcf_gate_params *p; - num_entries = to_gate(a)->param.num_entries; + p = rcu_dereference_protected(to_gate(a)->param, + lockdep_rtnl_is_held()); + num_entries = p->num_entries; return num_entries; } @@ -105,7 +121,8 @@ static inline struct action_gate_entry u32 num_entries; int i = 0; - p = &to_gate(a)->param; + p = rcu_dereference_protected(to_gate(a)->param, + lockdep_rtnl_is_held()); num_entries = p->num_entries; list_for_each_entry(entry, &p->entries, list) diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c index aacd57e5f4374..faaf34bcaff5d 100644 --- a/net/sched/act_gate.c +++ b/net/sched/act_gate.c @@ -34,7 +34,8 @@ static ktime_t gate_get_time(struct tcf_gate *gact) static void gate_get_start_time(struct tcf_gate *gact, ktime_t *start) { - struct tcf_gate_params *param = &gact->param; + struct tcf_gate_params *param = rcu_dereference_protected(gact->param, + lockdep_is_held(&gact->tcf_lock)); ktime_t now, base, cycle; u64 n; @@ -69,12 +70,14 @@ static enum hrtimer_restart gate_timer_func(struct hrtimer *timer) { struct tcf_gate *gact = container_of(timer, struct tcf_gate, hitimer); - struct tcf_gate_params *p = &gact->param; + struct tcf_gate_params *p; struct tcfg_gate_entry *next; ktime_t close_time, now; spin_lock(&gact->tcf_lock); + p = rcu_dereference_protected(gact->param, + lockdep_is_held(&gact->tcf_lock)); next = gact->next_entry; /* cycle start, clear pending bit, clear total octets */ @@ -274,18 +277,26 @@ static void gate_setup_timer(struct tcf_gate *gact, u64 basetime, enum tk_offsets tko, s32 clockid, bool do_init) { + struct tcf_gate_params *p; + if (!do_init) { - if (basetime == gact->param.tcfg_basetime && + p = rcu_dereference_protected(gact->param, + lockdep_is_held(&gact->tcf_lock)); + if (basetime == p->tcfg_basetime && tko == gact->tk_offset && - clockid == gact->param.tcfg_clockid) + clockid == p->tcfg_clockid) return; spin_unlock_bh(&gact->tcf_lock); hrtimer_cancel(&gact->hitimer); spin_lock_bh(&gact->tcf_lock); } - gact->param.tcfg_basetime = basetime; - gact->param.tcfg_clockid = clockid; + p = rcu_dereference_protected(gact->param, + lockdep_is_held(&gact->tcf_lock)); + if (p) { + p->tcfg_basetime = basetime; + p->tcfg_clockid = clockid; + } gact->tk_offset = tko; hrtimer_setup(&gact->hitimer, gate_timer_func, clockid, HRTIMER_MODE_ABS_SOFT); } @@ -376,15 +387,25 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla, gflags = nla_get_u32(tb[TCA_GATE_FLAGS]); gact = to_gate(*a); - if (ret == ACT_P_CREATED) - INIT_LIST_HEAD(&gact->param.entries); err = tcf_action_check_ctrlact(parm->action, tp, &goto_ch, extack); if (err < 0) goto release_idr; spin_lock_bh(&gact->tcf_lock); - p = &gact->param; + + if (ret == ACT_P_CREATED) { + p = kzalloc(sizeof(*p), GFP_ATOMIC); + if (!p) { + err = -ENOMEM; + goto chain_put; + } + INIT_LIST_HEAD(&p->entries); + rcu_assign_pointer(gact->param, p); + } else { + p = rcu_dereference_protected(gact->param, + lockdep_is_held(&gact->tcf_lock)); + } if (tb[TCA_GATE_CYCLE_TIME]) cycletime = nla_get_u64(tb[TCA_GATE_CYCLE_TIME]); @@ -446,21 +467,30 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla, * without taking tcf_lock. */ if (ret == ACT_P_CREATED) - gate_setup_timer(gact, gact->param.tcfg_basetime, - gact->tk_offset, gact->param.tcfg_clockid, + gate_setup_timer(gact, 0, + gact->tk_offset, 0, true); tcf_idr_release(*a, bind); return err; } +static void tcf_gate_params_free_rcu(struct rcu_head *head) +{ + struct tcf_gate_params *p = container_of(head, struct tcf_gate_params, rcu); + + release_entry_list(&p->entries); + kfree(p); +} + static void tcf_gate_cleanup(struct tc_action *a) { struct tcf_gate *gact = to_gate(a); struct tcf_gate_params *p; - p = &gact->param; + p = rcu_replace_pointer(gact->param, NULL, lockdep_rtnl_is_held()); hrtimer_cancel(&gact->hitimer); - release_entry_list(&p->entries); + if (p) + call_rcu(&p->rcu, tcf_gate_params_free_rcu); } static int dumping_entry(struct sk_buff *skb, @@ -512,7 +542,8 @@ static int tcf_gate_dump(struct sk_buff *skb, struct tc_action *a, spin_lock_bh(&gact->tcf_lock); opt.action = gact->tcf_action; - p = &gact->param; + p = rcu_dereference_protected(gact->param, + lockdep_is_held(&gact->tcf_lock)); if (nla_put(skb, TCA_GATE_PARMS, sizeof(opt), &opt)) goto nla_put_failure; -- 2.52.GIT