[PATCH net v3 1/7] net/sched: act_gate: zero-initialize netlink dump struct

[PATCH net v3 1/7] net/sched: act_gate: zero-initialize netlink dump struct

[Paul Moses]

Zero-initialize the dump struct before selective assignment to avoid
leaking stack padding in netlink replies. This matches other actions
(e.g. act_connmark) that zero-init their dump structs.

Fixes: a51c328df310 ("net: qos: introduce a gate control flow action")
Cc: stable@vger.kernel.org
Signed-off-by: Paul Moses <p@1g4.org>
---
 net/sched/act_gate.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
index c1f75f2727576..aacd57e5f4374 100644
--- a/net/sched/act_gate.c
+++ b/net/sched/act_gate.c
@@ -499,16 +499,16 @@ static int tcf_gate_dump(struct sk_buff *skb, struct tc_action *a,
 {
 	unsigned char *b = skb_tail_pointer(skb);
 	struct tcf_gate *gact = to_gate(a);
-	struct tc_gate opt = {
-		.index    = gact->tcf_index,
-		.refcnt   = refcount_read(&gact->tcf_refcnt) - ref,
-		.bindcnt  = atomic_read(&gact->tcf_bindcnt) - bind,
-	};
+	struct tc_gate opt = { };
 	struct tcfg_gate_entry *entry;
 	struct tcf_gate_params *p;
 	struct nlattr *entry_list;
 	struct tcf_t t;
 
+	opt.index = gact->tcf_index;
+	opt.refcnt = refcount_read(&gact->tcf_refcnt) - ref;
+	opt.bindcnt = atomic_read(&gact->tcf_bindcnt) - bind;
+
 	spin_lock_bh(&gact->tcf_lock);
 	opt.action = gact->tcf_action;
 
-- 
2.52.GIT

[PATCH net v3 2/7] net/sched: act_gate: add RCU support for parameter update

[Paul Moses]

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 <p@1g4.org>
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

[PATCH net v3 3/7] net/sched: act_gate: build schedule and RCU-swap

[Paul Moses]

Build a fresh params snapshot and swap it in with rcu_replace_pointer(),
then free the old snapshot via call_rcu(). This is the same publish+defer
pattern used in taprio sched swapping (sch_taprio.c, commit d5c4546062fd6f)
and in act_pedit param updates (act_pedit.c, commit 52cf89f78c01bf).

When REPLACE omits TCA_GATE_ENTRY_LIST, carry forward the old snapshot fields
(basetime/clockid/flags/cycletime/priority) and only override provided attrs,
so partial updates don’t reset unrelated state.

Parse entry lists with GFP_KERNEL and explicit error handling, matching taprio’s
schedule parsing (sch_taprio.c, commit 5a781ccbd19e46).

Fixes: a51c328df310 ("net: qos: introduce a gate control flow action")
Signed-off-by: Paul Moses <p@1g4.org>
Cc: stable@vger.kernel.org
---
 net/sched/act_gate.c | 185 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 140 insertions(+), 45 deletions(-)

diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
index faaf34bcaff5d..016708c10a8e0 100644
--- a/net/sched/act_gate.c
+++ b/net/sched/act_gate.c
@@ -32,10 +32,12 @@ static ktime_t gate_get_time(struct tcf_gate *gact)
 	return KTIME_MAX;
 }
 
-static void gate_get_start_time(struct tcf_gate *gact, ktime_t *start)
+static void tcf_gate_params_free_rcu(struct rcu_head *head);
+
+static void gate_get_start_time(struct tcf_gate *gact,
+				const struct tcf_gate_params *param,
+				ktime_t *start)
 {
-	struct tcf_gate_params *param = rcu_dereference_protected(gact->param,
-								  lockdep_is_held(&gact->tcf_lock));
 	ktime_t now, base, cycle;
 	u64 n;
 
@@ -228,13 +230,44 @@ static void release_entry_list(struct list_head *entries)
 	}
 }
 
+static int tcf_gate_copy_entries(struct tcf_gate_params *dst,
+				 const struct tcf_gate_params *src,
+				 struct netlink_ext_ack *extack)
+{
+	struct tcfg_gate_entry *entry;
+	int i = 0;
+
+	list_for_each_entry(entry, &src->entries, list) {
+		struct tcfg_gate_entry *new;
+
+		new = kzalloc(sizeof(*new), GFP_KERNEL);
+		if (!new) {
+			NL_SET_ERR_MSG(extack, "Not enough memory for entry");
+			return -ENOMEM;
+		}
+
+		new->index = entry->index;
+		new->gate_state = entry->gate_state;
+		new->interval = entry->interval;
+		new->ipv = entry->ipv;
+		new->maxoctets = entry->maxoctets;
+		INIT_LIST_HEAD(&new->list);
+		list_add_tail(&new->list, &dst->entries);
+		i++;
+	}
+
+	dst->num_entries = i;
+
+	return i;
+}
+
 static int parse_gate_list(struct nlattr *list_attr,
 			   struct tcf_gate_params *sched,
 			   struct netlink_ext_ack *extack)
 {
 	struct tcfg_gate_entry *entry;
 	struct nlattr *n;
-	int err, rem;
+	int err = -EINVAL, rem;
 	int i = 0;
 
 	if (!list_attr)
@@ -246,7 +279,7 @@ static int parse_gate_list(struct nlattr *list_attr,
 			continue;
 		}
 
-		entry = kzalloc(sizeof(*entry), GFP_ATOMIC);
+		entry = kzalloc(sizeof(*entry), GFP_KERNEL);
 		if (!entry) {
 			NL_SET_ERR_MSG(extack, "Not enough memory for entry");
 			err = -ENOMEM;
@@ -269,6 +302,7 @@ static int parse_gate_list(struct nlattr *list_attr,
 
 release_list:
 	release_entry_list(&sched->entries);
+	sched->num_entries = 0;
 
 	return err;
 }
@@ -291,12 +325,6 @@ static void gate_setup_timer(struct tcf_gate *gact, u64 basetime,
 		hrtimer_cancel(&gact->hitimer);
 		spin_lock_bh(&gact->tcf_lock);
 	}
-	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);
 }
@@ -307,20 +335,20 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
 			 struct netlink_ext_ack *extack)
 {
 	struct tc_action_net *tn = net_generic(net, act_gate_ops.net_id);
-	enum tk_offsets tk_offset = TK_OFFS_TAI;
-	bool bind = flags & TCA_ACT_FLAGS_BIND;
-	struct nlattr *tb[TCA_GATE_MAX + 1];
+	struct tcf_gate_params *p, *old_p = NULL;
 	struct tcf_chain *goto_ch = NULL;
-	u64 cycletime = 0, basetime = 0;
-	struct tcf_gate_params *p;
-	s32 clockid = CLOCK_TAI;
 	struct tcf_gate *gact;
 	struct tc_gate *parm;
-	int ret = 0, err;
-	u32 gflags = 0;
-	s32 prio = -1;
+	struct nlattr *tb[TCA_GATE_MAX + 1];
+	enum tk_offsets tk_offset = TK_OFFS_TAI;
+	u64 cycletime = 0, basetime = 0, cycletime_ext = 0;
 	ktime_t start;
+	s32 clockid = CLOCK_TAI;
+	s32 prio = -1;
+	u32 gflags = 0;
 	u32 index;
+	int ret = 0, err;
+	bool bind = flags & TCA_ACT_FLAGS_BIND;
 
 	if (!nla)
 		return -EINVAL;
@@ -388,32 +416,92 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
 
 	gact = to_gate(*a);
 
-	err = tcf_action_check_ctrlact(parm->action, tp, &goto_ch, extack);
-	if (err < 0)
+	p = kzalloc(sizeof(*p), GFP_KERNEL);
+	if (!p) {
+		err = -ENOMEM;
 		goto release_idr;
+	}
+	INIT_LIST_HEAD(&p->entries);
 
-	spin_lock_bh(&gact->tcf_lock);
+	if (!tb[TCA_GATE_ENTRY_LIST] && ret != ACT_P_CREATED) {
+		const struct tcf_gate_params *old_p_local;
 
-	if (ret == ACT_P_CREATED) {
-		p = kzalloc(sizeof(*p), GFP_ATOMIC);
-		if (!p) {
-			err = -ENOMEM;
-			goto chain_put;
+		old_p_local = rcu_dereference_protected(gact->param,
+							lockdep_rtnl_is_held());
+		if (!old_p_local) {
+			NL_SET_ERR_MSG(extack, "Missing schedule entries");
+			err = -EINVAL;
+			goto release_mem;
 		}
-		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_PRIORITY])
+			prio = old_p_local->tcfg_priority;
+
+		if (!tb[TCA_GATE_BASE_TIME])
+			basetime = old_p_local->tcfg_basetime;
+
+		if (!tb[TCA_GATE_FLAGS])
+			gflags = old_p_local->tcfg_flags;
+
+		if (!tb[TCA_GATE_CLOCKID]) {
+			clockid = old_p_local->tcfg_clockid;
+			switch (clockid) {
+			case CLOCK_REALTIME:
+				tk_offset = TK_OFFS_REAL;
+				break;
+			case CLOCK_MONOTONIC:
+				tk_offset = TK_OFFS_MAX;
+				break;
+			case CLOCK_BOOTTIME:
+				tk_offset = TK_OFFS_BOOT;
+				break;
+			case CLOCK_TAI:
+				tk_offset = TK_OFFS_TAI;
+				break;
+			default:
+				NL_SET_ERR_MSG(extack, "Invalid 'clockid'");
+				err = -EINVAL;
+				goto release_mem;
+			}
+		}
+
+		if (!tb[TCA_GATE_CYCLE_TIME])
+			cycletime = old_p_local->tcfg_cycletime;
+
+		if (!tb[TCA_GATE_CYCLE_TIME_EXT])
+			cycletime_ext = old_p_local->tcfg_cycletime_ext;
 	}
 
+	p->tcfg_priority = prio;
+	p->tcfg_flags = gflags;
+	p->tcfg_basetime = basetime;
+	p->tcfg_clockid = clockid;
+
 	if (tb[TCA_GATE_CYCLE_TIME])
 		cycletime = nla_get_u64(tb[TCA_GATE_CYCLE_TIME]);
 
 	if (tb[TCA_GATE_ENTRY_LIST]) {
 		err = parse_gate_list(tb[TCA_GATE_ENTRY_LIST], p, extack);
 		if (err < 0)
-			goto chain_put;
+			goto release_mem;
+	} else if (ret == ACT_P_CREATED) {
+		NL_SET_ERR_MSG(extack, "The entry list is empty");
+		err = -EINVAL;
+		goto release_mem;
+	} else {
+		const struct tcf_gate_params *old_p_local;
+
+		old_p_local = rcu_dereference_protected(gact->param,
+							lockdep_rtnl_is_held());
+		if (!old_p_local) {
+			NL_SET_ERR_MSG(extack, "Missing schedule entries");
+			err = -EINVAL;
+			goto release_mem;
+		}
+
+		err = tcf_gate_copy_entries(p, old_p_local, extack);
+		if (err < 0)
+			goto release_mem;
 	}
 
 	if (!cycletime) {
@@ -425,20 +513,26 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
 		cycletime = cycle;
 		if (!cycletime) {
 			err = -EINVAL;
-			goto chain_put;
+			goto release_mem;
 		}
 	}
 	p->tcfg_cycletime = cycletime;
 
 	if (tb[TCA_GATE_CYCLE_TIME_EXT])
-		p->tcfg_cycletime_ext =
-			nla_get_u64(tb[TCA_GATE_CYCLE_TIME_EXT]);
+		cycletime_ext = nla_get_u64(tb[TCA_GATE_CYCLE_TIME_EXT]);
+	p->tcfg_cycletime_ext = cycletime_ext;
 
+	err = tcf_action_check_ctrlact(parm->action, tp, &goto_ch, extack);
+	if (err < 0)
+		goto release_mem;
+
+	spin_lock_bh(&gact->tcf_lock);
 	gate_setup_timer(gact, basetime, tk_offset, clockid,
 			 ret == ACT_P_CREATED);
-	p->tcfg_priority = prio;
-	p->tcfg_flags = gflags;
-	gate_get_start_time(gact, &start);
+	gate_get_start_time(gact, p, &start);
+
+	old_p = rcu_replace_pointer(gact->param, p,
+				    lockdep_is_held(&gact->tcf_lock));
 
 	gact->current_close_time = start;
 	gact->current_gate_status = GATE_ACT_GATE_OPEN | GATE_ACT_PENDING;
@@ -455,13 +549,14 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
 	if (goto_ch)
 		tcf_chain_put_by_act(goto_ch);
 
-	return ret;
+	if (old_p)
+		call_rcu(&old_p->rcu, tcf_gate_params_free_rcu);
 
-chain_put:
-	spin_unlock_bh(&gact->tcf_lock);
+	return ret;
 
-	if (goto_ch)
-		tcf_chain_put_by_act(goto_ch);
+release_mem:
+	release_entry_list(&p->entries);
+	kfree(p);
 release_idr:
 	/* action is not inserted in any list: it's safe to init hitimer
 	 * without taking tcf_lock.
-- 
2.52.GIT

[PATCH net v3 4/7] net/sched: act_gate: read schedule via RCU

[Paul Moses]

Switch dump/accessor reads to RCU read-side sections. This matches other
actions that read params under rcu_read_lock(), e.g. act_tunnel_key dump
(commit e97ae742972f6c), act_ctinfo dump (commit 799c94178cf9c9), and
act_skbedit dump (commit 1f376373bd225c).

Dump reads tcf_action via READ_ONCE, following the lockless action reads used
in act_sample (commit 5c5670fae43027) and act_gact.

Timer logic stays under tcf_lock and uses rcu_dereference_protected(), keeping
RCU readers cheap while preserving lock-serialized timer updates.

Fixes: a51c328df310 ("net: qos: introduce a gate control flow action")
Signed-off-by: Paul Moses <p@1g4.org>
Cc: stable@vger.kernel.org
---
 include/net/tc_act/tc_gate.h | 38 +++++++++++++++++++++++-------------
 net/sched/act_gate.c         | 32 +++++++++++++++---------------
 2 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/include/net/tc_act/tc_gate.h b/include/net/tc_act/tc_gate.h
index 05968b3822392..9587d9e9fa38f 100644
--- a/include/net/tc_act/tc_gate.h
+++ b/include/net/tc_act/tc_gate.h
@@ -57,9 +57,10 @@ static inline s32 tcf_gate_prio(const struct tc_action *a)
 	s32 tcfg_prio;
 	struct tcf_gate_params *p;
 
-	p = rcu_dereference_protected(to_gate(a)->param,
-				      lockdep_rtnl_is_held());
+	rcu_read_lock();
+	p = rcu_dereference(to_gate(a)->param);
 	tcfg_prio = p->tcfg_priority;
+	rcu_read_unlock();
 
 	return tcfg_prio;
 }
@@ -69,9 +70,10 @@ static inline u64 tcf_gate_basetime(const struct tc_action *a)
 	u64 tcfg_basetime;
 	struct tcf_gate_params *p;
 
-	p = rcu_dereference_protected(to_gate(a)->param,
-				      lockdep_rtnl_is_held());
+	rcu_read_lock();
+	p = rcu_dereference(to_gate(a)->param);
 	tcfg_basetime = p->tcfg_basetime;
+	rcu_read_unlock();
 
 	return tcfg_basetime;
 }
@@ -81,9 +83,10 @@ static inline u64 tcf_gate_cycletime(const struct tc_action *a)
 	u64 tcfg_cycletime;
 	struct tcf_gate_params *p;
 
-	p = rcu_dereference_protected(to_gate(a)->param,
-				      lockdep_rtnl_is_held());
+	rcu_read_lock();
+	p = rcu_dereference(to_gate(a)->param);
 	tcfg_cycletime = p->tcfg_cycletime;
+	rcu_read_unlock();
 
 	return tcfg_cycletime;
 }
@@ -93,9 +96,10 @@ static inline u64 tcf_gate_cycletimeext(const struct tc_action *a)
 	u64 tcfg_cycletimeext;
 	struct tcf_gate_params *p;
 
-	p = rcu_dereference_protected(to_gate(a)->param,
-				      lockdep_rtnl_is_held());
+	rcu_read_lock();
+	p = rcu_dereference(to_gate(a)->param);
 	tcfg_cycletimeext = p->tcfg_cycletime_ext;
+	rcu_read_unlock();
 
 	return tcfg_cycletimeext;
 }
@@ -105,9 +109,10 @@ static inline u32 tcf_gate_num_entries(const struct tc_action *a)
 	u32 num_entries;
 	struct tcf_gate_params *p;
 
-	p = rcu_dereference_protected(to_gate(a)->param,
-				      lockdep_rtnl_is_held());
+	rcu_read_lock();
+	p = rcu_dereference(to_gate(a)->param);
 	num_entries = p->num_entries;
+	rcu_read_unlock();
 
 	return num_entries;
 }
@@ -121,19 +126,23 @@ static inline struct action_gate_entry
 	u32 num_entries;
 	int i = 0;
 
-	p = rcu_dereference_protected(to_gate(a)->param,
-				      lockdep_rtnl_is_held());
+	rcu_read_lock();
+	p = rcu_dereference(to_gate(a)->param);
 	num_entries = p->num_entries;
 
 	list_for_each_entry(entry, &p->entries, list)
 		i++;
 
-	if (i != num_entries)
+	if (i != num_entries) {
+		rcu_read_unlock();
 		return NULL;
+	}
 
 	oe = kcalloc(num_entries, sizeof(*oe), GFP_ATOMIC);
-	if (!oe)
+	if (!oe) {
+		rcu_read_unlock();
 		return NULL;
+	}
 
 	i = 0;
 	list_for_each_entry(entry, &p->entries, list) {
@@ -143,6 +152,7 @@ static inline struct action_gate_entry
 		oe[i].maxoctets = entry->maxoctets;
 		i++;
 	}
+	rcu_read_unlock();
 
 	return oe;
 }
diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
index 016708c10a8e0..da4802bbaf4ca 100644
--- a/net/sched/act_gate.c
+++ b/net/sched/act_gate.c
@@ -624,66 +624,66 @@ static int tcf_gate_dump(struct sk_buff *skb, struct tc_action *a,
 {
 	unsigned char *b = skb_tail_pointer(skb);
 	struct tcf_gate *gact = to_gate(a);
-	struct tc_gate opt = { };
 	struct tcfg_gate_entry *entry;
 	struct tcf_gate_params *p;
 	struct nlattr *entry_list;
+	struct tc_gate opt = { };
 	struct tcf_t t;
 
 	opt.index = gact->tcf_index;
 	opt.refcnt = refcount_read(&gact->tcf_refcnt) - ref;
 	opt.bindcnt = atomic_read(&gact->tcf_bindcnt) - bind;
 
-	spin_lock_bh(&gact->tcf_lock);
-	opt.action = gact->tcf_action;
-
-	p = rcu_dereference_protected(gact->param,
-				      lockdep_is_held(&gact->tcf_lock));
+	opt.action = READ_ONCE(gact->tcf_action);
 
 	if (nla_put(skb, TCA_GATE_PARMS, sizeof(opt), &opt))
 		goto nla_put_failure;
 
+	rcu_read_lock();
+	p = rcu_dereference(gact->param);
+
 	if (nla_put_u64_64bit(skb, TCA_GATE_BASE_TIME,
 			      p->tcfg_basetime, TCA_GATE_PAD))
-		goto nla_put_failure;
+		goto nla_put_failure_rcu;
 
 	if (nla_put_u64_64bit(skb, TCA_GATE_CYCLE_TIME,
 			      p->tcfg_cycletime, TCA_GATE_PAD))
-		goto nla_put_failure;
+		goto nla_put_failure_rcu;
 
 	if (nla_put_u64_64bit(skb, TCA_GATE_CYCLE_TIME_EXT,
 			      p->tcfg_cycletime_ext, TCA_GATE_PAD))
-		goto nla_put_failure;
+		goto nla_put_failure_rcu;
 
 	if (nla_put_s32(skb, TCA_GATE_CLOCKID, p->tcfg_clockid))
-		goto nla_put_failure;
+		goto nla_put_failure_rcu;
 
 	if (nla_put_u32(skb, TCA_GATE_FLAGS, p->tcfg_flags))
-		goto nla_put_failure;
+		goto nla_put_failure_rcu;
 
 	if (nla_put_s32(skb, TCA_GATE_PRIORITY, p->tcfg_priority))
-		goto nla_put_failure;
+		goto nla_put_failure_rcu;
 
 	entry_list = nla_nest_start_noflag(skb, TCA_GATE_ENTRY_LIST);
 	if (!entry_list)
-		goto nla_put_failure;
+		goto nla_put_failure_rcu;
 
 	list_for_each_entry(entry, &p->entries, list) {
 		if (dumping_entry(skb, entry) < 0)
-			goto nla_put_failure;
+			goto nla_put_failure_rcu;
 	}
 
 	nla_nest_end(skb, entry_list);
+	rcu_read_unlock();
 
 	tcf_tm_dump(&t, &gact->tcf_tm);
 	if (nla_put_64bit(skb, TCA_GATE_TM, sizeof(t), &t, TCA_GATE_PAD))
 		goto nla_put_failure;
-	spin_unlock_bh(&gact->tcf_lock);
 
 	return skb->len;
 
+nla_put_failure_rcu:
+	rcu_read_unlock();
 nla_put_failure:
-	spin_unlock_bh(&gact->tcf_lock);
 	nlmsg_trim(skb, b);
 	return -1;
 }
-- 
2.52.GIT

[PATCH net v3 5/7] net/sched: act_gate: cancel timer outside tcf_lock

[Paul Moses]

Move hrtimer_cancel() out from under tcf_lock, cancel only on clockid changes,
and always restart using the newly computed start time. For schedule
replacement, bypass the prior expiry clamp so basetime moves forward
without firing the new schedule early.

Other schedulers explicitly cancel hrtimers on reconfig/teardown, e.g.
sch_taprio advance_timer (commit 44d4775ca51805), sch_dualpi2 pi2_timer
(commit 320d031ad6e4d6), and qdisc_watchdog_cancel() (commit 2fbd3da3877ad8).

Fixes: a51c328df310 ("net: qos: introduce a gate control flow action")
Signed-off-by: Paul Moses <p@1g4.org>
Cc: stable@vger.kernel.org
---
 net/sched/act_gate.c | 52 ++++++++++++++++++++------------------------
 1 file changed, 24 insertions(+), 28 deletions(-)

diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
index da4802bbaf4ca..48ff378bb051a 100644
--- a/net/sched/act_gate.c
+++ b/net/sched/act_gate.c
@@ -55,15 +55,17 @@ static void gate_get_start_time(struct tcf_gate *gact,
 	*start = ktime_add_ns(base, (n + 1) * cycle);
 }
 
-static void gate_start_timer(struct tcf_gate *gact, ktime_t start)
+static void gate_start_timer(struct tcf_gate *gact, ktime_t start, bool replace)
 {
 	ktime_t expires;
 
-	expires = hrtimer_get_expires(&gact->hitimer);
-	if (expires == 0)
-		expires = KTIME_MAX;
+	if (!replace) {
+		expires = hrtimer_get_expires(&gact->hitimer);
+		if (expires == 0)
+			expires = KTIME_MAX;
 
-	start = min_t(ktime_t, start, expires);
+		start = min_t(ktime_t, start, expires);
+	}
 
 	hrtimer_start(&gact->hitimer, start, HRTIMER_MODE_ABS_SOFT);
 }
@@ -307,24 +309,9 @@ static int parse_gate_list(struct nlattr *list_attr,
 	return err;
 }
 
-static void gate_setup_timer(struct tcf_gate *gact, u64 basetime,
-			     enum tk_offsets tko, s32 clockid,
-			     bool do_init)
+static void gate_setup_timer(struct tcf_gate *gact,
+			     enum tk_offsets tko, s32 clockid)
 {
-	struct tcf_gate_params *p;
-
-	if (!do_init) {
-		p = rcu_dereference_protected(gact->param,
-					      lockdep_is_held(&gact->tcf_lock));
-		if (basetime == p->tcfg_basetime &&
-		    tko == gact->tk_offset &&
-		    clockid == p->tcfg_clockid)
-			return;
-
-		spin_unlock_bh(&gact->tcf_lock);
-		hrtimer_cancel(&gact->hitimer);
-		spin_lock_bh(&gact->tcf_lock);
-	}
 	gact->tk_offset = tko;
 	hrtimer_setup(&gact->hitimer, gate_timer_func, clockid, HRTIMER_MODE_ABS_SOFT);
 }
@@ -527,8 +514,19 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
 		goto release_mem;
 
 	spin_lock_bh(&gact->tcf_lock);
-	gate_setup_timer(gact, basetime, tk_offset, clockid,
-			 ret == ACT_P_CREATED);
+
+	if (ret == ACT_P_CREATED) {
+		gate_setup_timer(gact, tk_offset, clockid);
+	} else {
+		old_p = rcu_dereference_protected(gact->param,
+						  lockdep_is_held(&gact->tcf_lock));
+		if (!old_p || clockid != old_p->tcfg_clockid) {
+			spin_unlock_bh(&gact->tcf_lock);
+			hrtimer_cancel(&gact->hitimer);
+			spin_lock_bh(&gact->tcf_lock);
+			gate_setup_timer(gact, tk_offset, clockid);
+		}
+	}
 	gate_get_start_time(gact, p, &start);
 
 	old_p = rcu_replace_pointer(gact->param, p,
@@ -542,7 +540,7 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
 
 	goto_ch = tcf_action_set_ctrlact(*a, parm->action, goto_ch);
 
-	gate_start_timer(gact, start);
+	gate_start_timer(gact, start, ret != ACT_P_CREATED);
 
 	spin_unlock_bh(&gact->tcf_lock);
 
@@ -562,9 +560,7 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
 	 * without taking tcf_lock.
 	 */
 	if (ret == ACT_P_CREATED)
-		gate_setup_timer(gact, 0,
-				 gact->tk_offset, 0,
-				 true);
+		gate_setup_timer(gact, gact->tk_offset, 0);
 	tcf_idr_release(*a, bind);
 	return err;
 }
-- 
2.52.GIT

[PATCH net v3 6/7] net/sched: act_gate: reject empty schedule list

[Paul Moses]

Reject empty schedules (num_entries == 0) so next_entry is always valid and
RCU readers/timer logic never walk an empty list. taprio enforces the same
constraint on schedules (sch_taprio.c, commit 09dbdf28f9f9fa).

Fixes: a51c328df310 ("net: qos: introduce a gate control flow action")
Signed-off-by: Paul Moses <p@1g4.org>
Cc: stable@vger.kernel.org
---
 net/sched/act_gate.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
index 48ff378bb051a..e4134b9a4a314 100644
--- a/net/sched/act_gate.c
+++ b/net/sched/act_gate.c
@@ -509,6 +509,12 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
 		cycletime_ext = nla_get_u64(tb[TCA_GATE_CYCLE_TIME_EXT]);
 	p->tcfg_cycletime_ext = cycletime_ext;
 
+	if (p->num_entries == 0) {
+		NL_SET_ERR_MSG(extack, "The entry list is empty");
+		err = -EINVAL;
+		goto release_mem;
+	}
+
 	err = tcf_action_check_ctrlact(parm->action, tp, &goto_ch, extack);
 	if (err < 0)
 		goto release_mem;
-- 
2.52.GIT

[PATCH net v3 7/7] net/sched: act_gate: guard NULL params in accessors

[Paul Moses]

Guard NULL params in accessors/dump/timer paths to avoid crashes during
teardown or failed initialization. Other actions already guard params before
RCU cleanup (act_pedit, commit 52cf89f78c01bf; act_vlan, commits 4c5b9d9642c859
and 1edf8abe04090c), so act_gate should tolerate NULL in reader paths too.

Fixes: a51c328df310 ("net: qos: introduce a gate control flow action")
Signed-off-by: Paul Moses <p@1g4.org>
Cc: stable@vger.kernel.org
---
 include/net/tc_act/tc_gate.h | 30 ++++++++++++++++++++----------
 net/sched/act_gate.c         | 13 ++++++++++++-
 2 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/include/net/tc_act/tc_gate.h b/include/net/tc_act/tc_gate.h
index 9587d9e9fa38f..8c3309b0dd779 100644
--- a/include/net/tc_act/tc_gate.h
+++ b/include/net/tc_act/tc_gate.h
@@ -54,12 +54,13 @@ struct tcf_gate {
 
 static inline s32 tcf_gate_prio(const struct tc_action *a)
 {
-	s32 tcfg_prio;
+	s32 tcfg_prio = 0;
 	struct tcf_gate_params *p;
 
 	rcu_read_lock();
 	p = rcu_dereference(to_gate(a)->param);
-	tcfg_prio = p->tcfg_priority;
+	if (p)
+		tcfg_prio = p->tcfg_priority;
 	rcu_read_unlock();
 
 	return tcfg_prio;
@@ -67,12 +68,13 @@ 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;
+	u64 tcfg_basetime = 0;
 	struct tcf_gate_params *p;
 
 	rcu_read_lock();
 	p = rcu_dereference(to_gate(a)->param);
-	tcfg_basetime = p->tcfg_basetime;
+	if (p)
+		tcfg_basetime = p->tcfg_basetime;
 	rcu_read_unlock();
 
 	return tcfg_basetime;
@@ -80,12 +82,13 @@ 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;
+	u64 tcfg_cycletime = 0;
 	struct tcf_gate_params *p;
 
 	rcu_read_lock();
 	p = rcu_dereference(to_gate(a)->param);
-	tcfg_cycletime = p->tcfg_cycletime;
+	if (p)
+		tcfg_cycletime = p->tcfg_cycletime;
 	rcu_read_unlock();
 
 	return tcfg_cycletime;
@@ -93,12 +96,13 @@ 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;
+	u64 tcfg_cycletimeext = 0;
 	struct tcf_gate_params *p;
 
 	rcu_read_lock();
 	p = rcu_dereference(to_gate(a)->param);
-	tcfg_cycletimeext = p->tcfg_cycletime_ext;
+	if (p)
+		tcfg_cycletimeext = p->tcfg_cycletime_ext;
 	rcu_read_unlock();
 
 	return tcfg_cycletimeext;
@@ -106,12 +110,13 @@ 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;
+	u32 num_entries = 0;
 	struct tcf_gate_params *p;
 
 	rcu_read_lock();
 	p = rcu_dereference(to_gate(a)->param);
-	num_entries = p->num_entries;
+	if (p)
+		num_entries = p->num_entries;
 	rcu_read_unlock();
 
 	return num_entries;
@@ -128,6 +133,11 @@ static inline struct action_gate_entry
 
 	rcu_read_lock();
 	p = rcu_dereference(to_gate(a)->param);
+	if (!p) {
+		rcu_read_unlock();
+		return NULL;
+	}
+
 	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 e4134b9a4a314..65b53cbf37e67 100644
--- a/net/sched/act_gate.c
+++ b/net/sched/act_gate.c
@@ -82,7 +82,11 @@ static enum hrtimer_restart gate_timer_func(struct hrtimer *timer)
 
 	p = rcu_dereference_protected(gact->param,
 				      lockdep_is_held(&gact->tcf_lock));
+	if (!p)
+		goto out_unlock;
 	next = gact->next_entry;
+	if (!next)
+		goto out_unlock;
 
 	/* cycle start, clear pending bit, clear total octets */
 	gact->current_gate_status = next->gate_state ? GATE_ACT_GATE_OPEN : 0;
@@ -119,6 +123,11 @@ static enum hrtimer_restart gate_timer_func(struct hrtimer *timer)
 	spin_unlock(&gact->tcf_lock);
 
 	return HRTIMER_RESTART;
+
+out_unlock:
+	spin_unlock(&gact->tcf_lock);
+
+	return HRTIMER_NORESTART;
 }
 
 TC_INDIRECT_SCOPE int tcf_gate_act(struct sk_buff *skb,
@@ -584,8 +593,8 @@ static void tcf_gate_cleanup(struct tc_action *a)
 	struct tcf_gate *gact = to_gate(a);
 	struct tcf_gate_params *p;
 
-	p = rcu_replace_pointer(gact->param, NULL, lockdep_rtnl_is_held());
 	hrtimer_cancel(&gact->hitimer);
+	p = rcu_replace_pointer(gact->param, NULL, lockdep_rtnl_is_held());
 	if (p)
 		call_rcu(&p->rcu, tcf_gate_params_free_rcu);
 }
@@ -643,6 +652,8 @@ static int tcf_gate_dump(struct sk_buff *skb, struct tc_action *a,
 
 	rcu_read_lock();
 	p = rcu_dereference(gact->param);
+	if (!p)
+		goto nla_put_failure_rcu;
 
 	if (nla_put_u64_64bit(skb, TCA_GATE_BASE_TIME,
 			      p->tcfg_basetime, TCA_GATE_PAD))
-- 
2.52.GIT

Re: [PATCH net v3 1/7] net/sched: act_gate: zero-initialize netlink dump struct

[Eric Dumazet]

On Wed, Jan 21, 2026 at 2:20 PM Paul Moses <p@1g4.org> wrote:
>
> Zero-initialize the dump struct before selective assignment to avoid
> leaking stack padding in netlink replies. This matches other actions
> (e.g. act_connmark) that zero-init their dump structs.
>
> Fixes: a51c328df310 ("net: qos: introduce a gate control flow action")
> Cc: stable@vger.kernel.org
> Signed-off-by: Paul Moses <p@1g4.org>
> ---

I do not see a bug to fix, current code is fine.

act_connmark problem was that "struct tc_connmark" had a 16bit hole.

No such issue for struct tc_gate.

Re: [PATCH net v3 1/7] net/sched: act_gate: zero-initialize netlink dump struct

[Paul Moses]

Yes, it's not proven so you might be right, I knew it was 4 bytes at best. We can do next or toss it, I don't feel strongly either way.

On Wednesday, January 21st, 2026 at 7:25 AM, Eric Dumazet <edumazet@google.com> wrote:

> 
> 
> On Wed, Jan 21, 2026 at 2:20 PM Paul Moses p@1g4.org wrote:
> 
> > Zero-initialize the dump struct before selective assignment to avoid
> > leaking stack padding in netlink replies. This matches other actions
> > (e.g. act_connmark) that zero-init their dump structs.
> > 
> > Fixes: a51c328df310 ("net: qos: introduce a gate control flow action")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Paul Moses p@1g4.org
> > ---
> 
> 
> I do not see a bug to fix, current code is fine.
> 
> act_connmark problem was that "struct tc_connmark" had a 16bit hole.
> 
> No such issue for struct tc_gate.

Re: [PATCH net v3 1/7] net/sched: act_gate: zero-initialize netlink dump struct

[Eric Dumazet]

On Wed, Jan 21, 2026 at 2:39 PM Paul Moses <p@1g4.org> wrote:
>
> Yes, it's not proven so you might be right, I knew it was 4 bytes at best. We can do next or toss it, I don't feel strongly either way.
>

These bytes are cleared by C compilers.

https://en.cppreference.com/w/c/language/struct_initialization.html

Only holes might be left uninitialized.

> On Wednesday, January 21st, 2026 at 7:25 AM, Eric Dumazet <edumazet@google.com> wrote:
>
> >
> >
> > On Wed, Jan 21, 2026 at 2:20 PM Paul Moses p@1g4.org wrote:
> >
> > > Zero-initialize the dump struct before selective assignment to avoid
> > > leaking stack padding in netlink replies. This matches other actions
> > > (e.g. act_connmark) that zero-init their dump structs.
> > >
> > > Fixes: a51c328df310 ("net: qos: introduce a gate control flow action")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Paul Moses p@1g4.org
> > > ---
> >
> >
> > I do not see a bug to fix, current code is fine.
> >
> > act_connmark problem was that "struct tc_connmark" had a 16bit hole.
> >
> > No such issue for struct tc_gate.

Re: [PATCH net v3 1/7] net/sched: act_gate: zero-initialize netlink dump struct

[Paul Moses]

padding? why does fzero-init-padding-bits exist?




On Wednesday, January 21st, 2026 at 7:48 AM, Eric Dumazet <edumazet@google.com> wrote:

> 
> 
> On Wed, Jan 21, 2026 at 2:39 PM Paul Moses p@1g4.org wrote:
> 
> > Yes, it's not proven so you might be right, I knew it was 4 bytes at best. We can do next or toss it, I don't feel strongly either way.
> 
> 
> These bytes are cleared by C compilers.
> 
> https://en.cppreference.com/w/c/language/struct_initialization.html
> 
> Only holes might be left uninitialized.
> 
> > On Wednesday, January 21st, 2026 at 7:25 AM, Eric Dumazet edumazet@google.com wrote:
> > 
> > > On Wed, Jan 21, 2026 at 2:20 PM Paul Moses p@1g4.org wrote:
> > > 
> > > > Zero-initialize the dump struct before selective assignment to avoid
> > > > leaking stack padding in netlink replies. This matches other actions
> > > > (e.g. act_connmark) that zero-init their dump structs.
> > > > 
> > > > Fixes: a51c328df310 ("net: qos: introduce a gate control flow action")
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Paul Moses p@1g4.org
> > > > ---
> > > 
> > > I do not see a bug to fix, current code is fine.
> > > 
> > > act_connmark problem was that "struct tc_connmark" had a 16bit hole.
> > > 
> > > No such issue for struct tc_gate.

Re: [PATCH net v3 1/7] net/sched: act_gate: zero-initialize netlink dump struct

[Eric Dumazet]

On Wed, Jan 21, 2026 at 3:01 PM Paul Moses <p@1g4.org> wrote:
>
> padding? why does fzero-init-padding-bits exist?
>

Which padding are you referring to?

$ pahole -C tc_gate --hex  net/sched/act_gate.o
struct tc_gate {
__u32                      index;                /*     0   0x4 */
__u32                      capab;                /*   0x4   0x4 */
int                        action;               /*   0x8   0x4 */
int                        refcnt;               /*   0xc   0x4 */
int                        bindcnt;              /*  0x10   0x4 */

/* size: 20, cachelines: 1, members: 5 */
/* last cacheline: 20 bytes */
};

I see no padding.

>
>
>
> On Wednesday, January 21st, 2026 at 7:48 AM, Eric Dumazet <edumazet@google.com> wrote:
>
> >
> >
> > On Wed, Jan 21, 2026 at 2:39 PM Paul Moses p@1g4.org wrote:
> >
> > > Yes, it's not proven so you might be right, I knew it was 4 bytes at best. We can do next or toss it, I don't feel strongly either way.
> >
> >
> > These bytes are cleared by C compilers.
> >
> > https://en.cppreference.com/w/c/language/struct_initialization.html
> >
> > Only holes might be left uninitialized.
> >
> > > On Wednesday, January 21st, 2026 at 7:25 AM, Eric Dumazet edumazet@google.com wrote:
> > >
> > > > On Wed, Jan 21, 2026 at 2:20 PM Paul Moses p@1g4.org wrote:
> > > >
> > > > > Zero-initialize the dump struct before selective assignment to avoid
> > > > > leaking stack padding in netlink replies. This matches other actions
> > > > > (e.g. act_connmark) that zero-init their dump structs.
> > > > >
> > > > > Fixes: a51c328df310 ("net: qos: introduce a gate control flow action")
> > > > > Cc: stable@vger.kernel.org
> > > > > Signed-off-by: Paul Moses p@1g4.org
> > > > > ---
> > > >
> > > > I do not see a bug to fix, current code is fine.
> > > >
> > > > act_connmark problem was that "struct tc_connmark" had a 16bit hole.
> > > >
> > > > No such issue for struct tc_gate.

Re: [PATCH net v3 1/7] net/sched: act_gate: zero-initialize netlink dump struct

[Paul Moses]

Was looking at tcfg_gate_entry and tcf_gate along with this instance of nla_put, but I can't find my notes and I don't see the path currently. Like I said, remove it, I don't care.

    if (nla_put(skb, TCA_GATE_PARMS, sizeof(opt), &opt))
        goto nla_put_failure;


I have months invested into the UAF though and only just in the past 24 hours was able to stabilize into user space, so no effort whatsoever has been put into defeating kaslr or anything requiring infoleak.

Look forward to more input on the much larger issue at hand. 

Thanks
Paul




On Wednesday, January 21st, 2026 at 8:03 AM, Eric Dumazet <edumazet@google.com> wrote:

>
>
> On Wed, Jan 21, 2026 at 3:01 PM Paul Moses p@1g4.org wrote:
>
> > padding? why does fzero-init-padding-bits exist?
>
>
> Which padding are you referring to?
>
> $ pahole -C tc_gate --hex net/sched/act_gate.o
> struct tc_gate {
> __u32 index; /* 0 0x4 /
> __u32 capab; / 0x4 0x4 /
> int action; / 0x8 0x4 /
> int refcnt; / 0xc 0x4 /
> int bindcnt; / 0x10 0x4 /
>
> / size: 20, cachelines: 1, members: 5 /
> / last cacheline: 20 bytes */
> };
>
> I see no padding.
>
> > On Wednesday, January 21st, 2026 at 7:48 AM, Eric Dumazet edumazet@google.com wrote:
> >
> > > On Wed, Jan 21, 2026 at 2:39 PM Paul Moses p@1g4.org wrote:
> > >
> > > > Yes, it's not proven so you might be right, I knew it was 4 bytes at best. We can do next or toss it, I don't feel strongly either way.
> > >
> > > These bytes are cleared by C compilers.
> > >
> > > https://en.cppreference.com/w/c/language/struct_initialization.html
> > >
> > > Only holes might be left uninitialized.
> > >
> > > > On Wednesday, January 21st, 2026 at 7:25 AM, Eric Dumazet edumazet@google.com wrote:
> > > >
> > > > > On Wed, Jan 21, 2026 at 2:20 PM Paul Moses p@1g4.org wrote:
> > > > >
> > > > > > Zero-initialize the dump struct before selective assignment to avoid
> > > > > > leaking stack padding in netlink replies. This matches other actions
> > > > > > (e.g. act_connmark) that zero-init their dump structs.
> > > > > >
> > > > > > Fixes: a51c328df310 ("net: qos: introduce a gate control flow action")
> > > > > > Cc: stable@vger.kernel.org
> > > > > > Signed-off-by: Paul Moses p@1g4.org
> > > > > > ---
> > > > >
> > > > > I do not see a bug to fix, current code is fine.
> > > > >
> > > > > act_connmark problem was that "struct tc_connmark" had a 16bit hole.
> > > > >
> > > > > No such issue for struct tc_gate.

Re: [PATCH net v3 1/7] net/sched: act_gate: zero-initialize netlink dump struct

[Eric Dumazet]

On Wed, Jan 21, 2026 at 4:26 PM Paul Moses <p@1g4.org> wrote:
>
> Was looking at tcfg_gate_entry and tcf_gate along with this instance of nla_put, but I can't find my notes and I don't see the path currently. Like I said, remove it, I don't care.
>
>     if (nla_put(skb, TCA_GATE_PARMS, sizeof(opt), &opt))
>         goto nla_put_failure;

To be clear : we have the same pattern in a dozen of net/sched/act_*c files.
syzbot already found the problematic ones :

- net/sched/act_ife.c
- net/sched/act_connmark.c
- net/sched/act_ct.c
- net/sched/act_skbmod.c

We already checked all other files.

Unless another maintainer proves me wrong, let's remove this patch
from your series.

>
>
> I have months invested into the UAF though and only just in the past 24 hours was able to stabilize into user space, so no effort whatsoever has been put into defeating kaslr or anything requiring infoleak.
>
> Look forward to more input on the much larger issue at hand.

Sure, thanks for working on this.

Re: [PATCH net v3 2/7] net/sched: act_gate: add RCU support for parameter update

[Victor Nogueira]

On 21/01/2026 10:20, Paul Moses wrote:
> 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.
> [...]

I think you could've transformed patches 2, 3, 4 into a single patch.
Since all of them are RCU-related changes and they sometimes overwrite
each other.

Re: [PATCH net v3 3/7] net/sched: act_gate: build schedule and RCU-swap

[Victor Nogueira]

On 21/01/2026 10:20, Paul Moses wrote:
> Build a fresh params snapshot and swap it in with rcu_replace_pointer(),
> then free the old snapshot via call_rcu(). This is the same publish+defer
> pattern used in taprio sched swapping (sch_taprio.c, commit d5c4546062fd6f)
> and in act_pedit param updates (act_pedit.c, commit 52cf89f78c01bf).
> 
> When REPLACE omits TCA_GATE_ENTRY_LIST, carry forward the old snapshot fields
> (basetime/clockid/flags/cycletime/priority) and only override provided attrs,
> so partial updates don’t reset unrelated state.
> 
> Parse entry lists with GFP_KERNEL and explicit error handling, matching taprio’s
> schedule parsing (sch_taprio.c, commit 5a781ccbd19e46).
> [...]
> @@ -388,32 +416,92 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
>   
>   	gact = to_gate(*a);
>   
> -	err = tcf_action_check_ctrlact(parm->action, tp, &goto_ch, extack);
> -	if (err < 0)
> +	p = kzalloc(sizeof(*p), GFP_KERNEL);
> +	if (!p) {
> +		err = -ENOMEM;
>   		goto release_idr;
> +	}
> +	INIT_LIST_HEAD(&p->entries);
> [...]  
>   	if (!cycletime) {
> @@ -425,20 +513,26 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
>   		cycletime = cycle;
>   		if (!cycletime) {
>   			err = -EINVAL;
> -			goto chain_put;
> +			goto release_mem;
>   		}
>   	}
>   	p->tcfg_cycletime = cycletime;

You are always using the just allocated params pointer (p) when iterating
through the entries [1]. So cycletime will always be 0 when the user
doesn't specify it. This is breaking tdc:

not ok 1119 3719 - Replace gate base-time action
# Command exited with 255, expected 0
# RTNETLINK answers: Invalid argument
# We have an error talking to the kernel

[1] 
https://elixir.bootlin.com/linux/v6.18.6/source/net/sched/act_gate.c#L402

Re: [PATCH net v3 4/7] net/sched: act_gate: read schedule via RCU

[Victor Nogueira]

On 21/01/2026 10:20, Paul Moses wrote:
> Switch dump/accessor reads to RCU read-side sections. This matches other
> actions that read params under rcu_read_lock(), e.g. act_tunnel_key dump
> (commit e97ae742972f6c), act_ctinfo dump (commit 799c94178cf9c9), and
> act_skbedit dump (commit 1f376373bd225c).
> 
> Dump reads tcf_action via READ_ONCE, following the lockless action reads used
> in act_sample (commit 5c5670fae43027) and act_gact.
> 
> Timer logic stays under tcf_lock and uses rcu_dereference_protected(), keeping
> RCU readers cheap while preserving lock-serialized timer updates.
> 
> diff --git a/include/net/tc_act/tc_gate.h b/include/net/tc_act/tc_gate.h
> index 05968b3822392..9587d9e9fa38f 100644
> --- a/include/net/tc_act/tc_gate.h
> +++ b/include/net/tc_act/tc_gate.h
> @@ -57,9 +57,10 @@ static inline s32 tcf_gate_prio(const struct tc_action *a)
>   	s32 tcfg_prio;
>   	struct tcf_gate_params *p;
>   
> -	p = rcu_dereference_protected(to_gate(a)->param,
> -				      lockdep_rtnl_is_held());
> +	rcu_read_lock();
> +	p = rcu_dereference(to_gate(a)->param);
>   	tcfg_prio = p->tcfg_priority;
> +	rcu_read_unlock();

These helper functions are called with the tcf_lock acquired, so you
don't need rcu_read_lock here. You can just do:

p = rcu_dereference_protected(to_gate(a)->param,
			      lockdep_is_held(&gact->tcf_lock));

cheers,
Victor

Re: [PATCH net v3 6/7] net/sched: act_gate: reject empty schedule list

[Victor Nogueira]

On 21/01/2026 10:20, Paul Moses wrote:
> Reject empty schedules (num_entries == 0) so next_entry is always valid and
> RCU readers/timer logic never walk an empty list. taprio enforces the same
> constraint on schedules (sch_taprio.c, commit 09dbdf28f9f9fa).
> 
> Fixes: a51c328df310 ("net: qos: introduce a gate control flow action")
> Signed-off-by: Paul Moses <p@1g4.org>
> Cc: stable@vger.kernel.org
> ---
>   net/sched/act_gate.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
> index 48ff378bb051a..e4134b9a4a314 100644
> --- a/net/sched/act_gate.c
> +++ b/net/sched/act_gate.c
> @@ -509,6 +509,12 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
>   		cycletime_ext = nla_get_u64(tb[TCA_GATE_CYCLE_TIME_EXT]);
>   	p->tcfg_cycletime_ext = cycletime_ext;
>   
> +	if (p->num_entries == 0) {
> +		NL_SET_ERR_MSG(extack, "The entry list is empty");
> +		err = -EINVAL;
> +		goto release_mem;
> +	}

It would be simpler to check this in parse_gate_list.
That way you could return -EINVAL there directly
in case 0 entries were passed.

cheers,
Victor

Re: [PATCH net v3 7/7] net/sched: act_gate: guard NULL params in accessors

[Victor Nogueira]

On 21/01/2026 10:21, Paul Moses wrote:
> Guard NULL params in accessors/dump/timer paths to avoid crashes during
> teardown or failed initialization. Other actions already guard params before
> RCU cleanup (act_pedit, commit 52cf89f78c01bf; act_vlan, commits 4c5b9d9642c859
> and 1edf8abe04090c), so act_gate should tolerate NULL in reader paths too.
> [...]
> diff --git a/include/net/tc_act/tc_gate.h b/include/net/tc_act/tc_gate.h
> index 9587d9e9fa38f..8c3309b0dd779 100644
> --- a/include/net/tc_act/tc_gate.h
> +++ b/include/net/tc_act/tc_gate.h
> @@ -54,12 +54,13 @@ struct tcf_gate {
>   
>   static inline s32 tcf_gate_prio(const struct tc_action *a)
>   {
> -	s32 tcfg_prio;
> +	s32 tcfg_prio = 0;
>   	struct tcf_gate_params *p;
>   
>   	rcu_read_lock();
>   	p = rcu_dereference(to_gate(a)->param);
> -	tcfg_prio = p->tcfg_priority;
> +	if (p)
> +		tcfg_prio = p->tcfg_priority;

I don't believe you need to check for NULL in these helper functions. From
what I understood, the only place setting this to NULL is the cleanup
callback. You also won't be able to run this in parallel with the init
callback.

> [...]
>   	list_for_each_entry(entry, &p->entries, list)
> diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
> index e4134b9a4a314..65b53cbf37e67 100644
> --- a/net/sched/act_gate.c
> +++ b/net/sched/act_gate.c
> @@ -82,7 +82,11 @@ static enum hrtimer_restart gate_timer_func(struct hrtimer *timer)
>   
>   	p = rcu_dereference_protected(gact->param,
>   				      lockdep_is_held(&gact->tcf_lock));
> +	if (!p)
> +		goto out_unlock;

Also don't think you need to check this here.
Unless I'm missing something, cleanup will only set param to NULL after
the timer callback has finished executing.

> [...]
> @@ -643,6 +652,8 @@ static int tcf_gate_dump(struct sk_buff *skb, struct tc_action *a,
>   
>   	rcu_read_lock();
>   	p = rcu_dereference(gact->param);
> +	if (!p)
> +		goto nla_put_failure_rcu;

I don't think you need the check here either.
Take a look at act_vlan.

cheers,
Victor

Re: [PATCH net v3 6/7] net/sched: act_gate: reject empty schedule list

[Victor Nogueira]

On 21/01/2026 16:44, Victor Nogueira wrote:
> On 21/01/2026 10:20, Paul Moses wrote:
>> Reject empty schedules (num_entries == 0) so next_entry is always 
>> valid and
>> RCU readers/timer logic never walk an empty list. taprio enforces the 
>> same
>> constraint on schedules (sch_taprio.c, commit 09dbdf28f9f9fa).
>>
>> Fixes: a51c328df310 ("net: qos: introduce a gate control flow action")
>> Signed-off-by: Paul Moses <p@1g4.org>
>> Cc: stable@vger.kernel.org
>> ---
>>   net/sched/act_gate.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
>> index 48ff378bb051a..e4134b9a4a314 100644
>> --- a/net/sched/act_gate.c
>> +++ b/net/sched/act_gate.c
>> @@ -509,6 +509,12 @@ static int tcf_gate_init(struct net *net, struct 
>> nlattr *nla,
>>           cycletime_ext = nla_get_u64(tb[TCA_GATE_CYCLE_TIME_EXT]);
>>       p->tcfg_cycletime_ext = cycletime_ext;
>> +    if (p->num_entries == 0) {
>> +        NL_SET_ERR_MSG(extack, "The entry list is empty");
>> +        err = -EINVAL;
>> +        goto release_mem;
>> +    }
> 
> It would be simpler to check this in parse_gate_list.
> That way you could return -EINVAL there directly
> in case 0 entries were passed.

On second thought, I believe it would be better
to check whether parse_gate_list's return is 0
and the op is a create. Something like:

err = parse_gate_list(tb[TCA_GATE_ENTRY_LIST], p, extack);
...
if (!err && ret == ACT_P_CREATED) {
     NL_SET_ERR_MSG(extack, "The entry list is empty");
     err = -EINVAL;
     goto release_mem;
}

so that you don't need to add new arguments to
parse_gate_list.

cheers,
Victor

Re: [PATCH net v3 2/7] net/sched: act_gate: add RCU support for parameter update

[Paul Moses]

Yes, wanted to show logical flow in this iteration. 

Thanks
Paul




On Wednesday, January 21st, 2026 at 1:42 PM, Victor Nogueira <victor@mojatatu.com> wrote:

> 
> 
> On 21/01/2026 10:20, Paul Moses wrote:
> 
> > 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.
> > [...]
> 
> 
> I think you could've transformed patches 2, 3, 4 into a single patch.
> Since all of them are RCU-related changes and they sometimes overwrite
> each other.

Re: [PATCH net v3 6/7] net/sched: act_gate: reject empty schedule list

[Paul Moses]

Should REPLACE with an explicit entry list that yields 0 entries return -EINVAL or should it be treated the same as omitting TCA_GATE_ENTRY_LIST and keeping the old schedule?

thanks,
Paul





On Wednesday, January 21st, 2026 at 3:49 PM, Victor Nogueira <victor@mojatatu.com> wrote:

> 
> 
> On 21/01/2026 16:44, Victor Nogueira wrote:
> 
> > On 21/01/2026 10:20, Paul Moses wrote:
> > 
> > > Reject empty schedules (num_entries == 0) so next_entry is always
> > > valid and
> > > RCU readers/timer logic never walk an empty list. taprio enforces the
> > > same
> > > constraint on schedules (sch_taprio.c, commit 09dbdf28f9f9fa).
> > > 
> > > Fixes: a51c328df310 ("net: qos: introduce a gate control flow action")
> > > Signed-off-by: Paul Moses p@1g4.org
> > > Cc: stable@vger.kernel.org
> > > ---
> > > net/sched/act_gate.c | 6 ++++++
> > > 1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
> > > index 48ff378bb051a..e4134b9a4a314 100644
> > > --- a/net/sched/act_gate.c
> > > +++ b/net/sched/act_gate.c
> > > @@ -509,6 +509,12 @@ static int tcf_gate_init(struct net *net, struct
> > > nlattr *nla,
> > > cycletime_ext = nla_get_u64(tb[TCA_GATE_CYCLE_TIME_EXT]);
> > > p->tcfg_cycletime_ext = cycletime_ext;
> > > + if (p->num_entries == 0) {
> > > + NL_SET_ERR_MSG(extack, "The entry list is empty");
> > > + err = -EINVAL;
> > > + goto release_mem;
> > > + }
> > 
> > It would be simpler to check this in parse_gate_list.
> > That way you could return -EINVAL there directly
> > in case 0 entries were passed.
> 
> 
> On second thought, I believe it would be better
> to check whether parse_gate_list's return is 0
> and the op is a create. Something like:
> 
> err = parse_gate_list(tb[TCA_GATE_ENTRY_LIST], p, extack);
> ...
> if (!err && ret == ACT_P_CREATED) {
> NL_SET_ERR_MSG(extack, "The entry list is empty");
> err = -EINVAL;
> goto release_mem;
> }
> 
> so that you don't need to add new arguments to
> parse_gate_list.
> 
> cheers,
> Victor

Re: [PATCH net v3 6/7] net/sched: act_gate: reject empty schedule list

[Victor Nogueira]

On Mon, Jan 26, 2026 at 5:53 AM Paul Moses <p@1g4.org> wrote:
>
> Should REPLACE with an explicit entry list that yields 0 entries return -EINVAL or should it be treated the same as omitting TCA_GATE_ENTRY_LIST and keeping the old schedule?

It should be treated the same as omitting TCA_GATE_ENTRY_LIST and keeping
the old schedule.

cheers,
Victor

Re: [PATCH net v3 6/7] net/sched: act_gate: reject empty schedule list

[Paul Moses]

Ok, just to confirm the intended behavior changes compared to what is currently in the tree:

  create missing entry list      FAIL (got -22, expected 0)
  create empty entry list        FAIL (got -22, expected 0)
  replace append entries         REPLACE append failed: expected 2 entries, got 1
                                  FAIL (got -22, expected 0)

- CREATE with missing or empty entry list now returns -EINVAL  
  Previously, CREATE could appear to succeed if cycle_time was 
  provided even with no entries, but it still left an
  empty schedule and later called list_first_entry() at
  net/sched/act_gate.c:552, which is unsafe. Returning -EINVAL here is the
  correct behavior fix.

- REPLACE now replaces the schedule, it does not append  
  The old append behavior was an accident caused by reusing the same list and
  never clearing it. With the RCU snapshot change, a fresh schedule is built
  and swapped atomically, so providing a new entry list on REPLACE replaces
  the old one and avoids stale data.

- REPLACE with an empty entry list keeps the old schedule  

Thanks,
Paul

Re: [PATCH net v3 6/7] net/sched: act_gate: reject empty schedule list

[Victor Nogueira]

On Sat, Jan 31, 2026 at 12:00 PM Paul Moses <p@1g4.org> wrote:
>
> Ok, just to confirm the intended behavior changes compared to what is currently in the tree:
>
>   create missing entry list      FAIL (got -22, expected 0)
>   create empty entry list        FAIL (got -22, expected 0)
>   replace append entries         REPLACE append failed: expected 2 entries, got 1
>                                   FAIL (got -22, expected 0)
>
> - CREATE with missing or empty entry list now returns -EINVAL
>   Previously, CREATE could appear to succeed if cycle_time was
>   provided even with no entries, but it still left an
>   empty schedule and later called list_first_entry() at
>   net/sched/act_gate.c:552, which is unsafe. Returning -EINVAL here is the
>   correct behavior fix.
>
> - REPLACE now replaces the schedule, it does not append
>   The old append behavior was an accident caused by reusing the same list and
>   never clearing it. With the RCU snapshot change, a fresh schedule is built
>   and swapped atomically, so providing a new entry list on REPLACE replaces
>   the old one and avoids stale data.
>
> - REPLACE with an empty entry list keeps the old schedule

At first glance it looks ok.
However I can only be sure once you send it and I run some
tests. Just in case I am missing something.

cheers,
Victor