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

Paul Moses posted 7 patches 2 weeks, 4 days ago
[PATCH net v3 6/7] net/sched: act_gate: reject empty schedule list
Posted by Paul Moses 2 weeks, 4 days ago
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
Re: [PATCH net v3 6/7] net/sched: act_gate: reject empty schedule list
Posted by Victor Nogueira 2 weeks, 4 days ago
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 6/7] net/sched: act_gate: reject empty schedule list
Posted by Victor Nogueira 2 weeks, 4 days ago
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
Posted by Paul Moses 2 weeks ago
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
Posted by Victor Nogueira 1 week, 6 days ago
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
Posted by Paul Moses 1 week, 1 day ago
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
Posted by Victor Nogueira 6 days, 23 hours ago
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