linux-kernel - Re: [PATCH] sched: fix NULL pointer issue in pick_next

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170801083812.GH6524@worktop.programming.kicks-ass.net>
Date:   Tue, 1 Aug 2017 10:38:12 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Yafang Shao <laoar.shao@...il.com>
Cc:     mingo@...hat.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] sched: fix NULL pointer issue in pick_next_entity()

On Tue, Aug 01, 2017 at 06:01:56PM +0800, Yafang Shao wrote:
> When we select CFQ as the scheduler, in function pick_next_task_fair

CFS, CFQ is a block IO scheduler.

> it will pass NULL as the 2nd argument to pick_next_entity:
>     pick_next_entity(cfs_rq, NULL);
> 
> And once __pick_first_entity() is called, it could return NULL as well.
> 
> So in function pick_next_entity(), the local variable 'left' and 'curr'
> could both be NULL, then this will cause NULL pointer issue.
> 
> In order to fix this issue, we just need return NULL under the condition
> that both 'left' and 'curr' are NULL, meaning that no entity available.

And how would that happen? We only call pick_next_entity(.curr=NULL)
when we _know_ cfs_rq->nr_running.

> Signed-off-by: Yafang Shao <laoar.shao@...il.com>
> ---
>  kernel/sched/fair.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index c95880e..e64c359 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -3903,6 +3903,8 @@ static void clear_buddies(struct cfs_rq *cfs_rq, struct sched_entity *se)

You diff is broken, this is very much not the clear_buddies() function.

>  	struct sched_entity *left = __pick_first_entity(cfs_rq);
>  	struct sched_entity *se;
>  
> +	if (!left && !curr)
> +		return NULL;
>  	/*
>  	 * If curr is set we have to see if its left of the leftmost entity
>  	 * still in the tree, provided there was anything in the tree at all.
> -- 
> 1.8.3.1
>