net/sched: act_gate RCU schedule update fixes

Started by Paul Moses on Jan 21, 2026 · 26 messages from 3 authors · Last reply Feb 2, 2026

Series

7 patches, 2 files

+316 −140

Subsystems (3)

Maintainers (7)

Show 3 more
STALE284d

open the first message on its own page

  1. [PATCH net v3 1/7] net/sched: act_gate: zero-initialize netlink dump struct
    Related patches
  2. [PATCH net v3 4/7] net/sched: act_gate: read schedule via RCU
    Related patches
  3. [PATCH net v3 6/7] net/sched: act_gate: reject empty schedule list
    Related patches
  4. [PATCH net v3 5/7] net/sched: act_gate: cancel timer outside tcf_lock
    Related patches
  5. [PATCH net v3 3/7] net/sched: act_gate: build schedule and RCU-swap
    Related patches
  6. [PATCH net v3 7/7] net/sched: act_gate: guard NULL params in accessors
    Related patches
  7. [PATCH net v3 2/7] net/sched: act_gate: add RCU support for parameter update
    Related patches
Message details and related content
This series fixes a schedule lifetime race in `act_gate` between the
control-plane replace path and the running hrtimer callback (gate_timer_func).

Before this series, a gate action could be replaced via
`RTM_NEWACTION | NLM_F_REPLACE` while its hrtimer was armed or executing.
The replace path frees the existing schedule entries immediately, but
the timer callback may concurrently dereference and walk the old entry
list to compute the next expiry. This allows the timer to observe freed
memory, leading to UAF or NULL dereference depending on timing and reuse.

The fix publishes gate configuration updates using an RCU snapshot
pattern. A new parameter set (including the schedule list) is fully
constructed first and then swapped in atomically via an RCU-protected
pointer. Readers (timer, dump, and accessors) only dereference the
current snapshot under RCU, and the old snapshot is reclaimed via
`call_rcu()` after the swap. This ensures the timer never observes freed
or partially invalid schedules during a replace.

Additional hardening included in this series:

* Zero-initialize the netlink dump struct to avoid leaking stack padding
* Reject empty schedule lists, which result in undefined runtime behavior
* Guard against NULL parameters in accessors during teardown
* Cancel the hrtimer outside `tcf_lock` to avoid lock inversion with the
  timer callback; cancellation is limited to clockid changes to avoid
  unnecessary blocking

Stable relevance:

* Fixes UAF / NULL deref hazards when a gate action is replaced while the
  timer, dump, or accessors are active
* Fixes a potential deadlock when canceling the hrtimer under `tcf_lock`
* Fixes a padding information leak in netlink dumps
* Dependency chain: patches 2–7 build on patch 1 and on each other

Changes since v2:
* Split patch into logical steps
* Split hardening into explicit empty-schedule rejection and NULL
  accessor guards
* Align `tcf_gate_init()` initialization with net/sched conventions

21 more lines

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

Message details and related content
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: <redacted>
Signed-off-by: Paul Moses <redacted>
---
 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 4/7] net/sched: act_gate: read schedule via RCU

Message details and related content
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 <redacted>
Cc: <redacted>
---
 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 6/7] net/sched: act_gate: reject empty schedule list

Message details and related content
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 <redacted>
Cc: <redacted>
---
 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 5/7] net/sched: act_gate: cancel timer outside tcf_lock

Message details and related content
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 <redacted>
Cc: <redacted>
---
 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 3/7] net/sched: act_gate: build schedule and RCU-swap

Message details and related content
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 <redacted>
Cc: <redacted>
---
 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 7/7] net/sched: act_gate: guard NULL params in accessors

Message details and related content
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 <redacted>
Cc: <redacted>
---
 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

Message details and related content
On Wed, Jan 21, 2026 at 2:20 PM Paul Moses [off-list ref] 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: <redacted>
Signed-off-by: Paul Moses <redacted>
---
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.

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

Message details and related content
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 <redacted>
Cc: <redacted>
---
 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

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

Message details and related content
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 [off-list ref] wrote:

On Wed, Jan 21, 2026 at 2:20 PM Paul Moses p@1g4.org wrote:
quoted
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: <redacted>
Signed-off-by: Paul Moses <redacted>
---

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

Message details and related content
On Wed, Jan 21, 2026 at 2:39 PM Paul Moses [off-list ref] 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 [off-list ref] wrote:
quoted

On Wed, Jan 21, 2026 at 2:20 PM Paul Moses p@1g4.org wrote:
quoted
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: <redacted>
Signed-off-by: Paul Moses <redacted>
---

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

Message details and related content
padding? why does fzero-init-padding-bits exist?




On Wednesday, January 21st, 2026 at 7:48 AM, Eric Dumazet [off-list ref] wrote:

On Wed, Jan 21, 2026 at 2:39 PM Paul Moses p@1g4.org wrote:
quoted
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.
quoted
On Wednesday, January 21st, 2026 at 7:25 AM, Eric Dumazet edumazet@google.com wrote:
quoted
On Wed, Jan 21, 2026 at 2:20 PM Paul Moses p@1g4.org wrote:
quoted
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: <redacted>
Signed-off-by: Paul Moses <redacted>
---
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

Message details and related content
On Wed, Jan 21, 2026 at 3:01 PM Paul Moses [off-list ref] 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 [off-list ref] wrote:
quoted

On Wed, Jan 21, 2026 at 2:39 PM Paul Moses p@1g4.org wrote:
quoted
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.
quoted
On Wednesday, January 21st, 2026 at 7:25 AM, Eric Dumazet edumazet@google.com wrote:
quoted
On Wed, Jan 21, 2026 at 2:20 PM Paul Moses p@1g4.org wrote:
quoted
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: <redacted>
Signed-off-by: Paul Moses <redacted>
---
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

Message details and related content
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 [off-list ref] wrote:

On Wed, Jan 21, 2026 at 3:01 PM Paul Moses p@1g4.org wrote:
quoted
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.
quoted
On Wednesday, January 21st, 2026 at 7:48 AM, Eric Dumazet edumazet@google.com wrote:
quoted
On Wed, Jan 21, 2026 at 2:39 PM Paul Moses p@1g4.org wrote:
quoted
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.
quoted
On Wednesday, January 21st, 2026 at 7:25 AM, Eric Dumazet edumazet@google.com wrote:
quoted
On Wed, Jan 21, 2026 at 2:20 PM Paul Moses p@1g4.org wrote:
quoted
Zero-initialize the dump struct before selective assignment to avoid

13 more quoted lines

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

Message details and related content
On Wed, Jan 21, 2026 at 4:26 PM Paul Moses [off-list ref] 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

Message details and related content
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

Message details and related content
On 21/01/2026 10:20, Paul Moses wrote:
quoted hunk
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

Message details and related content
On 21/01/2026 10:20, Paul Moses wrote:
quoted hunk
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

Message details and related content
On 21/01/2026 10:20, Paul Moses wrote:
quoted hunk
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 <redacted>
Cc: <redacted>
---
  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

Message details and related content
On 21/01/2026 10:21, Paul Moses wrote:
quoted hunk
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.
quoted hunk
[...]
  	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.
quoted hunk
[...]
@@ -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

Message details and related content
On 21/01/2026 16:44, Victor Nogueira wrote:
On 21/01/2026 10:20, Paul Moses wrote:
quoted
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 <redacted>
Cc: <redacted>
---
  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

Message details and related content
Yes, wanted to show logical flow in this iteration. 

Thanks
Paul




On Wednesday, January 21st, 2026 at 1:42 PM, Victor Nogueira [off-list ref] wrote:

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

Message details and related content
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 [off-list ref] wrote:

On 21/01/2026 16:44, Victor Nogueira wrote:
quoted
On 21/01/2026 10:20, Paul Moses wrote:
quoted
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 <redacted>
Cc: <redacted>
---
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

15 more quoted lines

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

Message details and related content
On Mon, Jan 26, 2026 at 5:53 AM Paul Moses [off-list ref] 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

Message details and related content
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

Message details and related content
On Sat, Jan 31, 2026 at 12:00 PM Paul Moses [off-list ref] 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
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help