Messages in this thread | | | Date | Thu, 1 Apr 2010 00:41:47 -0700 (PDT) | From | David Rientjes <> | Subject | Re: [PATCH] oom: fix the unsafe proc_oom_score()->badness() call |
| |
On Wed, 31 Mar 2010, Oleg Nesterov wrote:
> But. Oh well. David, oom-badness-heuristic-rewrite.patch changed badness() > to consult p->signal->oom_score_adj. Until recently this was wrong when it > is called from proc_oom_score(). > > This means oom-badness-heuristic-rewrite.patch depends on > signals-make-task_struct-signal-immutable-refcountable.patch, or we > need the pid_alive() check again. >
oom-badness-heuristic-rewrite.patch didn't change anything, Linus' tree currently dereferences p->signal->oom_adj which is no different from dereferencing p->signal->oom_score_adj without a refcount on the signal_struct in -mm. oom_adj was moved to struct signal_struct in 2.6.32, see 28b83c5.
> oom_badness() gets the new argument, long totalpages, and the callers > were updated. However, long uptime is not used any longer, probably > it make sense to kill this arg and simplify the callers? Unless you > are going to take run-time into account later. > > So, I think -mm needs the patch below, but I have no idea how to > write the changelog ;) > > Oleg. > > --- x/fs/proc/base.c > +++ x/fs/proc/base.c > @@ -430,12 +430,13 @@ static const struct file_operations proc > /* The badness from the OOM killer */ > static int proc_oom_score(struct task_struct *task, char *buffer) > { > - unsigned long points; > + unsigned long points = 0; > struct timespec uptime; > > do_posix_clock_monotonic_gettime(&uptime); > read_lock(&tasklist_lock); > - points = oom_badness(task->group_leader, > + if (pid_alive(task)) > + points = oom_badness(task, > global_page_state(NR_INACTIVE_ANON) + > global_page_state(NR_ACTIVE_ANON) + > global_page_state(NR_INACTIVE_FILE) +
This should be protected by the get_proc_task() on the inode before this function is called from proc_info_read().
| |