From: Peter Zijlstra <peterz@infradead.org>
To: shenghui <crosslonelyover@gmail.com>
Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org,
Greg KH <greg@kroah.com>,
linux-mm@kvack.org, mingo@elte.hu
Subject: Re: [PATCH] avoid return NULL on root rb_node in rb_next/rb_prev in lib/rbtree.c
Date: Mon, 28 Jun 2010 15:55:20 +0200 [thread overview]
Message-ID: <1277733320.3561.50.camel@laptop> (raw)
In-Reply-To: <AANLkTilJDrpoFGyTSrKg3Hg59u9TvBLbxk4HAVKBvjxQ@mail.gmail.com>
On Mon, 2010-06-28 at 21:17 +0800, shenghui wrote:
> Hi,
>
> I'm reading cfs code, and get the following potential bug.
>
> In kernel/sched_fair.c, we can get the following call thread:
>
> 1778static struct task_struct *pick_next_task_fair(struct rq *rq)
> 1779{
> ...
> 1787 do {
> 1788 se = pick_next_entity(cfs_rq);
> 1789 set_next_entity(cfs_rq, se);
> 1790 cfs_rq = group_cfs_rq(se);
> 1791 } while (cfs_rq);
> ...
> 1797}
>
> 925static struct sched_entity *pick_next_entity(struct cfs_rq *cfs_rq)
> 926{
> 927 struct sched_entity *se = __pick_next_entity(cfs_rq);
> ...
> 941 return se;
> 942}
>
> 377static struct sched_entity *__pick_next_entity(struct cfs_rq *cfs_rq)
> 378{
> 379 struct rb_node *left = cfs_rq->rb_leftmost;
> 380
> 381 if (!left)
> 382 return NULL;
> ...
> 385}
>
> To manipulate cfs_rq->rb_leftmost, __dequeue_entity does the following:
>
> 365static void __dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se)
> 366{
> 367 if (cfs_rq->rb_leftmost == &se->run_node) {
> 368 struct rb_node *next_node;
> 369
> 370 next_node = rb_next(&se->run_node);
> 371 cfs_rq->rb_leftmost = next_node;
> 372 }
> 373
> 374 rb_erase(&se->run_node, &cfs_rq->tasks_timeline);
> 375}
>
> Here, if se->run_node is the root rb_node, next_node will be set NULL
> by rb_next.
> Then __pick_next_entity may get NULL on some call, and set_next_entity
> may deference
> NULL value.
So if ->rb_leftmost is NULL, then the if (!left) check in
__pick_next_entity() would return null.
As to the NULL deref in in pick_next_task_fair()->set_next_entity() that
should never happen because pick_next_task_fair() will bail
on !->nr_running.
Furthermore, you've failed to mention what kernel version you're looking
at.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2010-06-28 13:55 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-06-28 13:17 shenghui
2010-06-28 13:55 ` Peter Zijlstra [this message]
2010-06-28 23:48 ` shenghui
2010-06-29 6:34 ` shenghui
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1277733320.3561.50.camel@laptop \
--to=peterz@infradead.org \
--cc=crosslonelyover@gmail.com \
--cc=greg@kroah.com \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mingo@elte.hu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox