Commit fb66df20 authored by Vladimir Oltean's avatar Vladimir Oltean Committed by Jakub Kicinski
Browse files

net/sched: taprio: extend minimum interval restriction to entire cycle too 



It is possible for syzbot to side-step the restriction imposed by the
blamed commit in the Fixes: tag, because the taprio UAPI permits a
cycle-time different from (and potentially shorter than) the sum of
entry intervals.

We need one more restriction, which is that the cycle time itself must
be larger than N * ETH_ZLEN bit times, where N is the number of schedule
entries. This restriction needs to apply regardless of whether the cycle
time came from the user or was the implicit, auto-calculated value, so
we move the existing "cycle == 0" check outside the "if "(!new->cycle_time)"
branch. This way covers both conditions and scenarios.

Add a selftest which illustrates the issue triggered by syzbot.

Fixes: b5b73b26 ("taprio: Fix allowing too small intervals")
Reported-by: default avatar <syzbot+a7d2b1d5d1af83035567@syzkaller.appspotmail.com>
Closes: https://lore.kernel.org/netdev/0000000000007d66bc06196e7c66@google.com/


Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
Link: https://lore.kernel.org/r/20240527153955.553333-2-vladimir.oltean@nxp.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent e6341341

net/sched/sch_taprio.c

+5 −5
Original line number Diff line number Diff line
@@ -1151,11 +1151,6 @@ static int parse_taprio_schedule(struct taprio_sched *q, struct nlattr **tb, 
		list_for_each_entry(entry, &new->entries, list)

			cycle = ktime_add_ns(cycle, entry->interval);



		if (!cycle) {

			NL_SET_ERR_MSG(extack, "'cycle_time' can never be 0");

			return -EINVAL;

		}



		if (cycle < 0 || cycle > INT_MAX) {

			NL_SET_ERR_MSG(extack, "'cycle_time' is too big");

			return -EINVAL;
@@ -1164,6 +1159,11 @@ static int parse_taprio_schedule(struct taprio_sched *q, struct nlattr **tb, 
		new->cycle_time = cycle;

	}



	if (new->cycle_time < new->num_entries * length_to_duration(q, ETH_ZLEN)) {

		NL_SET_ERR_MSG(extack, "'cycle_time' is too small");

		return -EINVAL;

	}



	taprio_calculate_gate_durations(q, new);



	return 0;

tools/testing/selftests/tc-testing/tc-tests/qdiscs/taprio.json

+22 −0
Original line number Diff line number Diff line
@@ -154,6 +154,28 @@ 
            "echo \"1\" > /sys/bus/netdevsim/del_device"

        ]

    },

    {

        "id": "831f",

        "name": "Add taprio Qdisc with too short cycle-time",

        "category": [

            "qdisc",

            "taprio"

        ],

        "plugins": {

            "requires": "nsPlugin"

        },

        "setup": [

            "echo \"1 1 8\" > /sys/bus/netdevsim/new_device"

        ],

        "cmdUnderTest": "$TC qdisc add dev $ETH root handle 1: taprio num_tc 2 queues 1@0 1@1 sched-entry S 01 200000 sched-entry S 02 200000 cycle-time 100 clockid CLOCK_TAI",

        "expExitCode": "2",

        "verifyCmd": "$TC qdisc show dev $ETH",

        "matchPattern": "qdisc taprio 1: root refcnt",

        "matchCount": "0",

        "teardown": [

            "echo \"1\" > /sys/bus/netdevsim/del_device"

        ]

    },

    {

        "id": "3e1e",

        "name": "Add taprio Qdisc with an invalid cycle-time",