net/sched: taprio: fix use-after-free in advance_sched() on schedule switch

In advance_sched(), when should_change_schedules() returns true,
switch_schedules() is called to promote the admin schedule to oper.
switch_schedules() queues the old oper schedule for RCU freeing via
call_rcu(), but 'next' still points into an entry of the old oper
schedule. The subsequent 'next->end_time = end_time' and
rcu_assign_pointer(q->current_entry, next) are use-after-free.

Fix this by selecting 'next' from the new oper schedule immediately
after switch_schedules(), and using its pre-calculated end_time.
setup_first_end_time() sets the first entry's end_time to
base_time + interval when the schedule is installed, so the value
is already correct.

The deleted 'end_time = sched_base_time(admin)' assignment was also
harmful independently: it would overwrite the new first entry's
pre-calculated end_time with just base_time.

Fixes: a3d43c0d56f1 ("taprio: Add support adding an admin schedule")
Reported-by: Junxi Qian <qjx1298677004@gmail.com>
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index 8e375281195061da848fb2bfaf79cf125afccac0..a47a09d764001d027fb26ed723c9fea0b68aa83c 100644 (file)
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -972,11 +972,12 @@ static enum hrtimer_restart advance_sched(struct hrtimer *timer)
        }
 
        if (should_change_schedules(admin, oper, end_time)) {
-               /* Set things so the next time this runs, the new
-                * schedule runs.
-                */
-               end_time = sched_base_time(admin);
                switch_schedules(q, &admin, &oper);
+               /* After changing schedules, the next entry is the first one
+                * in the new schedule, with a pre-calculated end_time.
+                */
+               next = list_first_entry(&oper->entries, struct sched_entry, list);
+               end_time = next->end_time;
        }
 
        next->end_time = end_time;