2021-12-09 03:32:36

by Jiasheng Jiang

Subject: [PATCH] sched/uclamp: potential dereference of null pointer

The return value of kzalloc() needs to be checked.
To avoid use of null pointer in case of the failure of alloc.

Fixes: 391e43da797a ("sched: Move all scheduler bits into kernel/sched/")
Signed-off-by: Jiasheng Jiang <[email protected]>
---
kernel/sched/core.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index e5858999b54d..988a4a372e14 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -8125,6 +8125,8 @@ void __init sched_init(void)
#endif
if (ptr) {
ptr = (unsigned long)kzalloc(ptr, GFP_NOWAIT);
+ if (!ptr)
+ return;

#ifdef CONFIG_FAIR_GROUP_SCHED
root_task_group.se = (struct sched_entity **)ptr;
--
2.25.1

2021-12-09 15:23:21

by Steven Rostedt

Subject: Re: [PATCH] sched/uclamp: potential dereference of null pointer

On Thu, 9 Dec 2021 11:31:27 +0800
Jiasheng Jiang <[email protected]> wrote:

> The return value of kzalloc() needs to be checked.
> To avoid use of null pointer in case of the failure of alloc.
>
> Fixes: 391e43da797a ("sched: Move all scheduler bits into kernel/sched/")
> Signed-off-by: Jiasheng Jiang <[email protected]>
> ---
> kernel/sched/core.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index e5858999b54d..988a4a372e14 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -8125,6 +8125,8 @@ void __init sched_init(void)
> #endif
> if (ptr) {
> ptr = (unsigned long)kzalloc(ptr, GFP_NOWAIT);
> + if (!ptr)
> + return;

If this were to happen the system would crash immediately, with or without
the return.

If you are worried about not being able to allocate ptr, then the only
reasonable fix here is

BUG_ON(!ptr);

That way you would know exactly why your system crashed. Because just
returning would crash for other reasons and make it less debuggable.

-- Steve


>
> #ifdef CONFIG_FAIR_GROUP_SCHED
> root_task_group.se = (struct sched_entity **)ptr;

2021-12-09 15:25:23

by Steven Rostedt

Subject: Re: [PATCH] sched/uclamp: potential dereference of null pointer

On Thu, 9 Dec 2021 10:23:13 -0500
Steven Rostedt <[email protected]> wrote:

> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -8125,6 +8125,8 @@ void __init sched_init(void)
> > #endif
> > if (ptr) {
> > ptr = (unsigned long)kzalloc(ptr, GFP_NOWAIT);
> > + if (!ptr)
> > + return;
>
> If this were to happen the system would crash immediately, with or without
> the return.
>
> If you are worried about not being able to allocate ptr, then the only
> reasonable fix here is
>
> BUG_ON(!ptr);
>
> That way you would know exactly why your system crashed. Because just
> returning would crash for other reasons and make it less debuggable.

And this is not worth the churn (because if it failed to allocate, you have
bigger problems to deal with).

So NAK on this change.

-- Steve