Syzkaller reports general protection fault at reweight_entity() in 5.10
stable releases. The problem has been fixed by the following patch which
can be applied with minor changes to the 5.10 branch.
Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
From: Tadeusz Struk <[email protected]>
commit 13765de8148f71fa795e0a6607de37c49ea5915a upstream.
Syzbot found a GPF in reweight_entity. This has been bisected to
commit 4ef0c5c6b5ba ("kernel/sched: Fix sched_fork() access an invalid
sched_task_group")
There is a race between sched_post_fork() and setpriority(PRIO_PGRP)
within a thread group that causes a null-ptr-deref in
reweight_entity() in CFS. The scenario is that the main process spawns
number of new threads, which then call setpriority(PRIO_PGRP, 0, -20),
wait, and exit. For each of the new threads the copy_process() gets
invoked, which adds the new task_struct and calls sched_post_fork()
for it.
In the above scenario there is a possibility that
setpriority(PRIO_PGRP) and set_one_prio() will be called for a thread
in the group that is just being created by copy_process(), and for
which the sched_post_fork() has not been executed yet. This will
trigger a null pointer dereference in reweight_entity(), as it will
try to access the run queue pointer, which hasn't been set.
Before the mentioned change the cfs_rq pointer for the task has been
set in sched_fork(), which is called much earlier in copy_process(),
before the new task is added to the thread_group. Now it is done in
the sched_post_fork(), which is called after that. To fix the issue
the remove the update_load param from the update_load param() function
and call reweight_task() only if the task flag doesn't have the
TASK_NEW flag set.
Fixes: 4ef0c5c6b5ba ("kernel/sched: Fix sched_fork() access an invalid sched_task_group")
Reported-by: [email protected]
Signed-off-by: Tadeusz Struk <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Reviewed-by: Dietmar Eggemann <[email protected]>
Cc: [email protected]
Link: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Fedor Pchelkin <[email protected]>
---
5.10 adaptation: Replaced a task_struct field '__state' with 'state'
kernel/sched/core.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index e437d946b27b..86c5e08b393b 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -844,8 +844,9 @@ int tg_nop(struct task_group *tg, void *data)
}
#endif
-static void set_load_weight(struct task_struct *p, bool update_load)
+static void set_load_weight(struct task_struct *p)
{
+ bool update_load = !(READ_ONCE(p->state) & TASK_NEW);
int prio = p->static_prio - MAX_RT_PRIO;
struct load_weight *load = &p->se.load;
@@ -3262,7 +3263,7 @@ int sched_fork(unsigned long clone_flags, struct task_struct *p)
p->static_prio = NICE_TO_PRIO(0);
p->prio = p->normal_prio = p->static_prio;
- set_load_weight(p, false);
+ set_load_weight(p);
/*
* We don't need the reset flag anymore after the fork. It has
@@ -5011,7 +5012,7 @@ void set_user_nice(struct task_struct *p, long nice)
put_prev_task(rq, p);
p->static_prio = NICE_TO_PRIO(nice);
- set_load_weight(p, true);
+ set_load_weight(p);
old_prio = p->prio;
p->prio = effective_prio(p);
@@ -5184,7 +5185,7 @@ static void __setscheduler_params(struct task_struct *p,
*/
p->rt_priority = attr->sched_priority;
p->normal_prio = normal_prio(p);
- set_load_weight(p, true);
+ set_load_weight(p);
}
/*
@@ -7189,7 +7190,7 @@ void __init sched_init(void)
atomic_set(&rq->nr_iowait, 0);
}
- set_load_weight(&init_task, false);
+ set_load_weight(&init_task);
/*
* The boot idle thread does lazy MMU switching as well:
--
2.25.1
On Fri, Aug 19, 2022 at 10:11:40AM +0300, Fedor Pchelkin wrote:
> From: Tadeusz Struk <[email protected]>
>
> commit 13765de8148f71fa795e0a6607de37c49ea5915a upstream.
>
> Syzbot found a GPF in reweight_entity. This has been bisected to
> commit 4ef0c5c6b5ba ("kernel/sched: Fix sched_fork() access an invalid
> sched_task_group")
>
> There?is a race between sched_post_fork() and setpriority(PRIO_PGRP)
> within a thread group that causes a null-ptr-deref?in
> reweight_entity() in CFS. The scenario is that the main process spawns
> number of new threads, which then call setpriority(PRIO_PGRP, 0, -20),
> wait, and exit. For each of the new threads the copy_process() gets
> invoked, which adds the new task_struct and calls sched_post_fork()
> for it.
>
> In the above scenario there is a possibility that
> setpriority(PRIO_PGRP) and set_one_prio() will be called for a thread
> in the group that is just being created by copy_process(), and for
> which the sched_post_fork() has not been executed yet. This will
> trigger a null pointer dereference in reweight_entity(),?as it will
> try to access the run queue pointer, which hasn't been set.
>
> Before the mentioned change the cfs_rq pointer for the task has been
> set in sched_fork(), which is called much earlier in copy_process(),
> before the new task is added to the thread_group. Now it is done in
> the sched_post_fork(), which is called after that. To fix the issue
> the remove the update_load param from the update_load param() function
> and call reweight_task() only if the task flag doesn't have the
> TASK_NEW flag set.
>
> Fixes: 4ef0c5c6b5ba ("kernel/sched: Fix sched_fork() access an invalid sched_task_group")
> Reported-by: [email protected]
> Signed-off-by: Tadeusz Struk <[email protected]>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> Reviewed-by: Dietmar Eggemann <[email protected]>
> Cc: [email protected]
> Link: https://lkml.kernel.org/r/[email protected]
> Signed-off-by: Fedor Pchelkin <[email protected]>
> ---
> 5.10 adaptation: Replaced a task_struct field '__state' with 'state'
Now queued up, thanks.
greg k-h