Messages in this thread Patch in this message | | | Date | Thu, 9 Jun 2016 09:33:24 +0800 | From | Yuyang Du <> | Subject | Re: Divide-by-zero in post_init_entity_util_avg |
| |
On Thu, Jun 09, 2016 at 10:01:42AM +0100, Chris Wilson wrote: > I've presumed commit 2b8c41daba327 ("sched/fair: Initiate a new task's > util avg to a bounded value") to be at fault, hence the CCs. Though it > may just be a victim. > > gdb says 0x43/0x80 is > > 725 if (cfs_rq->avg.util_avg != 0) { > 726 sa->util_avg = cfs_rq->avg.util_avg * se->load.weight; > -> 727 sa->util_avg /= (cfs_rq->avg.load_avg + 1); > 728 > 729 if (sa->util_avg > cap) > 730 sa->util_avg = cap; > 731 } else { > > I've run the same fork-heavy workload that seemed to hit the initial > fault under kasan. kasan has not reported any errors, nor has the bug > reoccurred after a day (earlier I had a couple of panics within a few > hours). > > Is it possible for a race window where cfg_rq->avg.load_avg is indeed > -1? Any evidence of other memcorruption in the above?
-1 should not be possible, sounds like a soft error.
But, a race is anyway hazardous. Thanks a lot, Chris.
-- Subject: [PATCH] sched/fair: Avoid hazardous reading cfs_rq->avg.load_avg without rq lock
The commit 2b8c41daba327 ("sched/fair: Initiate a new task's util avg to a bounded value") references cfs_rq->avg.load_avg and then the value is used as a divisor (actually cfs_rq->avg.load_avg + 1).
This race condition may cause a divide-by-zero exception. Fix it by moving it into rq locked section.
Reported-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Yuyang Du <yuyang.du@intel.com> --- kernel/sched/core.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 385c947..b9f44df 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -2535,10 +2535,9 @@ void wake_up_new_task(struct task_struct *p) */ set_task_cpu(p, select_task_rq(p, task_cpu(p), SD_BALANCE_FORK, 0)); #endif + rq = __task_rq_lock(p, &rf); /* Post initialize new task's util average when its cfs_rq is set */ post_init_entity_util_avg(&p->se); - - rq = __task_rq_lock(p, &rf); activate_task(rq, p, 0); p->on_rq = TASK_ON_RQ_QUEUED; trace_sched_wakeup_new(p); --
| |