linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] sched/fair: Fix overflow in vruntime_eligible() causing NULL return
@ 2025-07-09  9:38 Zicheng Qu
  2025-07-09 11:53 ` Peter Zijlstra
  2025-08-07  3:06 ` Fang Xiang
  0 siblings, 2 replies; 6+ messages in thread
From: Zicheng Qu @ 2025-07-09  9:38 UTC (permalink / raw)
  To: mingo, peterz, juri.lelli, vincent.guittot, dietmar.eggemann,
	rostedt, bsegall, mgorman, vschneid, linux-kernel
  Cc: tanghui20, zhangqiao22, judy.chenhui, quzicheng

In the vruntime_eligible() function, the original comparison:
	return avg >= (s64)(vruntime - cfs_rq->min_vruntime) * load;
could produce incorrect results due to integer overflow in the 'avg' or
s64 part.

This overflow causes the comparison to return false even when the
mathematical result should be true, leading all tasks to be falsely
deemed ineligible. Consequently, pick_eevdf() returns NULL, triggering a
kernel crash.

The best approach should be to dig deep into why overflow occurs, which
attributes lead to the overflow, whether it is normal, and how to avoid
it. Not pretty sure if this part of the modification will introduce new
issues, but it may be incorrect to simply spot a potentially
overflowing integer type and directly use the >= sign for comparison.
Therefore, drawing on the enqueue method in the rb tree, to avoid the
impact of overflow, the method of first performing subtraction and then
comparing with 0 is also adopted.

The following are the relevant attributes that cause the return of NULL:
crash> struct cfs_rq.avg_vruntime,avg_load,min_vruntime
ffff9ea77ecb4280
  avg_vruntime = -4392414779907141680,
  avg_load = 28644,
  min_vruntime = 239776551994501,

sched_entity: curr
crash> struct sched_entity.deadline,min_vruntime,vruntime,load,vlag
0xffff9ea74510d800
  deadline = 86432035728535,
  min_vruntime = 86431486021988,
  vruntime = 86431531134184,
  load = {
    weight = 6066,
    inv_weight = 0
  },
  vlag = 86431823023195,

the sched_entity mapping to the root node in the cfs_rq
crash> struct sched_entity.deadline,min_vruntime,vruntime,load,vlag -l
sched_entity.run_node 0xffff9ea849fbc350
  deadline = 18446615868453340281,
  min_vruntime = 18446615868452621390,
  vruntime = 18446615868453018539,
  load = {
    weight = 9777152,
    inv_weight = 449829
  },
  vlag = -5599,

the sched_entity mapping to the leftmost node in the csf_rq, also the
left child of root node.
crash> struct sched_entity.deadline,min_vruntime,vruntime,load,vlag -l
sched_entity.run_node 0xffff9ea78d0280d0
  deadline = 18446615868452943132,
  min_vruntime = 18446615868452621390,
  vruntime = 18446615868452621390,
  load = {
    weight = 9777152,
    inv_weight = 449829
  },
  vlag = 444143,

the sched_entity mapping to the rightmost node in the csf_rq, also the
right child of root node.
crash> struct sched_entity.deadline,min_vruntime,vruntime,load,vlag -l
sched_entity.run_node 0xffff9ea783144350
  deadline = 515705106937888,
  min_vruntime = 515705106616146,
  vruntime = 515705106616146,
  load = {
    weight = 9777152,
    inv_weight = 449829
  },
  vlag = -260493,

Fixes: 147f3efaa241 ("sched/fair: Implement an EEVDF-like scheduling policy")
Signed-off-by: Zicheng Qu <quzicheng@huawei.com>
---
 kernel/sched/fair.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 7a14da5396fb..bfa4090cef93 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -734,7 +734,7 @@ static int vruntime_eligible(struct cfs_rq *cfs_rq, u64 vruntime)
 		load += weight;
 	}
 
-	return avg >= (s64)(vruntime - cfs_rq->min_vruntime) * load;
+	return avg - (s64)(vruntime - cfs_rq->min_vruntime) * load >= 0;
 }
 
 int entity_eligible(struct cfs_rq *cfs_rq, struct sched_entity *se)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] sched/fair: Fix overflow in vruntime_eligible() causing NULL return
  2025-07-09  9:38 Zicheng Qu
@ 2025-07-09 11:53 ` Peter Zijlstra
  2025-07-10  2:01   ` Zicheng Qu
  2025-08-07  3:06 ` Fang Xiang
  1 sibling, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2025-07-09 11:53 UTC (permalink / raw)
  To: Zicheng Qu
  Cc: mingo, juri.lelli, vincent.guittot, dietmar.eggemann, rostedt,
	bsegall, mgorman, vschneid, linux-kernel, tanghui20, zhangqiao22,
	judy.chenhui

On Wed, Jul 09, 2025 at 09:38:29AM +0000, Zicheng Qu wrote:

> The best approach should be to dig deep into why overflow occurs, which
> attributes lead to the overflow, whether it is normal, and how to avoid
> it.

What kernel version are you seeing this on?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] sched/fair: Fix overflow in vruntime_eligible() causing NULL return
  2025-07-09 11:53 ` Peter Zijlstra
@ 2025-07-10  2:01   ` Zicheng Qu
  2025-07-10 11:13     ` Peter Zijlstra
  0 siblings, 1 reply; 6+ messages in thread
From: Zicheng Qu @ 2025-07-10  2:01 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: mingo, juri.lelli, vincent.guittot, dietmar.eggemann, rostedt,
	bsegall, mgorman, vschneid, linux-kernel, tanghui20, zhangqiao22,
	judy.chenhui

Hi,

Based on LTS 6.6. I also looked at the code logic around 
vruntime_eligible() in the mainline. It seems that if 
vruntime_eligible() consistently returned false, this could lead to null 
pointer dereferences.

I'm wondering if it's feasible to adjust the handling of integer 
overflows within vruntime_eligible() the way shown in my patch, or if 
there's a specific design rationale behind directly comparing integers 
that are susceptible to overflow in the current implementation?

Thanks

On 7/9/2025 7:53 PM, Peter Zijlstra wrote:
> On Wed, Jul 09, 2025 at 09:38:29AM +0000, Zicheng Qu wrote:
>
>> The best approach should be to dig deep into why overflow occurs, which
>> attributes lead to the overflow, whether it is normal, and how to avoid
>> it.
> What kernel version are you seeing this on?
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] sched/fair: Fix overflow in vruntime_eligible() causing NULL return
  2025-07-10  2:01   ` Zicheng Qu
@ 2025-07-10 11:13     ` Peter Zijlstra
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2025-07-10 11:13 UTC (permalink / raw)
  To: Zicheng Qu
  Cc: mingo, juri.lelli, vincent.guittot, dietmar.eggemann, rostedt,
	bsegall, mgorman, vschneid, linux-kernel, tanghui20, zhangqiao22,
	judy.chenhui

On Thu, Jul 10, 2025 at 10:01:15AM +0800, Zicheng Qu wrote:
> Hi,
> 
> Based on LTS 6.6. I also looked at the code logic around vruntime_eligible()
> in the mainline. It seems that if vruntime_eligible() consistently returned
> false, this could lead to null pointer dereferences.

That is a truly ancient kernel.  Please verify the issue is reproducible
on something recent.

Specifically, commit bbce3de72be5 ("sched/eevdf: Fix se->slice being set
to U64_MAX and resulting crash") cured something very similar to this.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] sched/fair: Fix overflow in vruntime_eligible() causing NULL return
  2025-07-09  9:38 Zicheng Qu
  2025-07-09 11:53 ` Peter Zijlstra
@ 2025-08-07  3:06 ` Fang Xiang
  1 sibling, 0 replies; 6+ messages in thread
From: Fang Xiang @ 2025-08-07  3:06 UTC (permalink / raw)
  To: Zicheng Qu
  Cc: mingo, peterz, juri.lelli, vincent.guittot, dietmar.eggemann,
	rostedt, bsegall, mgorman, vschneid, linux-kernel, tanghui20,
	zhangqiao22, judy.chenhui

On Wed, Jul 09, 2025 at 09:38:29AM +0000, Zicheng Qu wrote:
> In the vruntime_eligible() function, the original comparison:
> 	return avg >= (s64)(vruntime - cfs_rq->min_vruntime) * load;
> could produce incorrect results due to integer overflow in the 'avg' or
> s64 part.
> 
> This overflow causes the comparison to return false even when the
> mathematical result should be true, leading all tasks to be falsely
> deemed ineligible. Consequently, pick_eevdf() returns NULL, triggering a
> kernel crash.
> 
> The best approach should be to dig deep into why overflow occurs, which
> attributes lead to the overflow, whether it is normal, and how to avoid
> it. Not pretty sure if this part of the modification will introduce new
> issues, but it may be incorrect to simply spot a potentially
> overflowing integer type and directly use the >= sign for comparison.
> Therefore, drawing on the enqueue method in the rb tree, to avoid the
> impact of overflow, the method of first performing subtraction and then
> comparing with 0 is also adopted.
> 
> The following are the relevant attributes that cause the return of NULL:
> crash> struct cfs_rq.avg_vruntime,avg_load,min_vruntime
> ffff9ea77ecb4280
>   avg_vruntime = -4392414779907141680,
>   avg_load = 28644,
>   min_vruntime = 239776551994501,
> 
> sched_entity: curr
> crash> struct sched_entity.deadline,min_vruntime,vruntime,load,vlag
> 0xffff9ea74510d800
>   deadline = 86432035728535,
>   min_vruntime = 86431486021988,
>   vruntime = 86431531134184,
>   load = {
>     weight = 6066,
>     inv_weight = 0
>   },
>   vlag = 86431823023195,
> 
> the sched_entity mapping to the root node in the cfs_rq
> crash> struct sched_entity.deadline,min_vruntime,vruntime,load,vlag -l
> sched_entity.run_node 0xffff9ea849fbc350
>   deadline = 18446615868453340281,
>   min_vruntime = 18446615868452621390,
>   vruntime = 18446615868453018539,
>   load = {
>     weight = 9777152,
>     inv_weight = 449829
>   },
>   vlag = -5599,
> 
> the sched_entity mapping to the leftmost node in the csf_rq, also the
> left child of root node.
> crash> struct sched_entity.deadline,min_vruntime,vruntime,load,vlag -l
> sched_entity.run_node 0xffff9ea78d0280d0
>   deadline = 18446615868452943132,
>   min_vruntime = 18446615868452621390,
>   vruntime = 18446615868452621390,
>   load = {
>     weight = 9777152,
>     inv_weight = 449829
>   },
>   vlag = 444143,
> 
> the sched_entity mapping to the rightmost node in the csf_rq, also the
> right child of root node.
> crash> struct sched_entity.deadline,min_vruntime,vruntime,load,vlag -l
> sched_entity.run_node 0xffff9ea783144350
>   deadline = 515705106937888,
>   min_vruntime = 515705106616146,
>   vruntime = 515705106616146,
>   load = {
>     weight = 9777152,
>     inv_weight = 449829
>   },
>   vlag = -260493,
> 
> Fixes: 147f3efaa241 ("sched/fair: Implement an EEVDF-like scheduling policy")
> Signed-off-by: Zicheng Qu <quzicheng@huawei.com>
> ---
>  kernel/sched/fair.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 7a14da5396fb..bfa4090cef93 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -734,7 +734,7 @@ static int vruntime_eligible(struct cfs_rq *cfs_rq, u64 vruntime)
>  		load += weight;
>  	}
>  
> -	return avg >= (s64)(vruntime - cfs_rq->min_vruntime) * load;
> +	return avg - (s64)(vruntime - cfs_rq->min_vruntime) * load >= 0;
>  }
>  
>  int entity_eligible(struct cfs_rq *cfs_rq, struct sched_entity *se)
> -- 
> 2.34.1
> 
May I ask which specific version of the 6.6 LTS branch are you using? 
On the 6.6.54 kernel version, I've also encountered the same issue in my device.
The following are the relevant attributes that cause the return of NULL in pick_eevdf():
cfs_rq:
  avg_vruntime = -34534164181975319,
  avg_load = 1531079,
  min_vruntime = 26052230711305,
  nr_running = 245,
  h_nr_running = 245,

sched_entity: curr is NULL

the sched_entity mapping to the root node in the cfs_rq
  deadline = 26052232578140,
  min_vruntime = 17382361763776,
  vruntime = 26052229578140,
  load = {
    weight = 1048576,
    inv_weight = 4194304
  },
  vlag = 656101,

the sched_entity mapping to the leftmost node in the csf_rq
  deadline = 17382364763776,
  min_vruntime = 17382361763776,
  vruntime = 17382361763776,
  load = {
    weight = 1048576,
    inv_weight = 4194304
  },
  vlag = 8648818863863,

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] sched/fair: Fix overflow in vruntime_eligible() causing NULL return
@ 2025-08-08  9:21 quzicheng
  0 siblings, 0 replies; 6+ messages in thread
From: quzicheng @ 2025-08-08  9:21 UTC (permalink / raw)
  To: Fang Xiang
  Cc: mingo@redhat.com, peterz@infradead.org, juri.lelli@redhat.com,
	vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
	rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
	vschneid@redhat.com, linux-kernel@vger.kernel.org, tanghui (C),
	Zhangqiao (2012 lab), Chenhui (Judy)

> May I ask which specific version of the 6.6 LTS branch are you using?
Hi, based on 6.6.31

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-08-08  9:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-08  9:21 [PATCH] sched/fair: Fix overflow in vruntime_eligible() causing NULL return quzicheng
  -- strict thread matches above, loose matches on Subject: below --
2025-07-09  9:38 Zicheng Qu
2025-07-09 11:53 ` Peter Zijlstra
2025-07-10  2:01   ` Zicheng Qu
2025-07-10 11:13     ` Peter Zijlstra
2025-08-07  3:06 ` Fang Xiang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).