2021-10-25 12:21:58

by Jiasheng Jiang

[permalink] [raw]
Subject: [PATCH v2] sched: Fix implicit type conversion

The variable 'n' is defined as ULONG. However in the cpumask_next(),
it is used as INT.
That is vulnerable and may cause overflow.
For example, if the value of 'n' is (2^31 - 1), then it can pass the
check ('n == 0') and ('n-- > 0'). Then in cpumask_next(), its value
is (2^31 - 3). But it is implicit type conversed to int, its actual
value is -3, which is illegal in the cpumask_next().
Therefore, it might be better to define 'n' as INT.

Fixes: cb152ff ("sched: Fix /proc/sched_stat failure on very very large systems")
Signed-off-by: Jiasheng Jiang <[email protected]>
---
kernel/sched/stats.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/stats.c b/kernel/sched/stats.c
index 3f93fc3..6503d3a 100644
--- a/kernel/sched/stats.c
+++ b/kernel/sched/stats.c
@@ -82,7 +82,7 @@ static int show_schedstat(struct seq_file *seq, void *v)
*/
static void *schedstat_start(struct seq_file *file, loff_t *offset)
{
- unsigned long n = *offset;
+ int n = *offset;

if (n == 0)
return (void *) 1;
--
2.7.4


2021-10-25 18:07:34

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH v2] sched: Fix implicit type conversion

On Mon, Oct 25, 2021 at 12:19:37PM +0000, Jiasheng Jiang wrote:
> The variable 'n' is defined as ULONG. However in the cpumask_next(),
> it is used as INT.
> That is vulnerable and may cause overflow.
> For example, if the value of 'n' is (2^31 - 1), then it can pass the

That would need nr_cpu_ids to be that large. How's that going to happen?

2021-10-25 21:21:17

by Jiasheng Jiang

[permalink] [raw]
Subject: Re: Re: [PATCH v2] sched: Fix implicit type conversion

On Mon, Oct 25, 2021 at 12:43:55AM +0000, Peter Zijlstra wrote:
>> The variable 'n' is defined as ULONG. However in the cpumask_next(),
>> it is used as INT.
>> That is vulnerable and may cause overflow.
>> For example, if the value of 'n' is (2^31 - 1), then it can pass the

>That would need nr_cpu_ids to be that large. How's that going to happen?

First, maybe it is hard to exploit it now, but who knows the future.
Second, the patch cost much less than the loss caused by the unexpected
input.
Third, it is universally accepted that the implicit type conversion is
vulnerable. Therefore, it will set an example for others that having
the good programming custom.