* [Patch V2] proc: check error pointer returned by m_start() [not found] <AANLkTi=mqKF=xeMYbj9LZgPfSLHSkO1JRXsNTW3LyHqu@mail.gmail.com> @ 2011-03-28 5:26 ` Amerigo Wang 2011-03-28 5:45 ` Cong Wang 2011-03-28 5:46 ` Anca Emanuel 0 siblings, 2 replies; 4+ messages in thread From: Amerigo Wang @ 2011-03-28 5:26 UTC (permalink / raw) To: linux-kernel Cc: WANG Cong, Linus Torvalds, Al Viro, WANG Cong, Andrew Morton, Eric B Munson, David Rientjes, Dave Hansen, Mel Gorman, linux-fsdevel From: WANG Cong <xiyou.wangcong@gmail.com> V2: move the check into m_stop() as suggested by Linus, also, most ->show() implementations assume the second parameter 'v' is not NULL, this fixes them too. Anca reported a bug: [15117.080119] BUG: unable to handle kernel paging request at fffffffffffffff3 [15117.080152] IP: [<ffffffff811b4989>] vma_stop+0x19/0x40 Linus did the initial analysis, and found this was caused by commit ec6fd8a4355c ("report errors in /proc/*/*map* sanely"), which replaces NULL with various ERR_PTR() cases. This is true, that commit changed the return value of m_start(), which will return an error pointer on failure, but Al forgot to check the error pointer in m_stop() which will be called when m_start() fails. This patches fixes it. Reported-by: Anca Emanuel <anca.emanuel@gmail.com> Tested-by: Anca Emanuel <anca.emanuel@gmail.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: WANG Cong <amwang@redhat.com> Signed-off-by: WANG Cong <xiyou.wangcong@gmail.com> --- diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c index 7c708a4..8e59169 100644 --- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c @@ -124,8 +124,10 @@ static void *m_start(struct seq_file *m, loff_t *pos) return ERR_PTR(-ESRCH); mm = mm_for_maps(priv->task); - if (!mm || IS_ERR(mm)) + if (IS_ERR_OR_NULL(mm)) { + put_task_struct(priv->task); return mm; + } down_read(&mm->mmap_sem); tail_vma = get_gate_vma(priv->task->mm); @@ -182,6 +184,8 @@ static void m_stop(struct seq_file *m, void *v) struct proc_maps_private *priv = m->private; struct vm_area_struct *vma = v; + if (IS_ERR_OR_NULL(v)) + return; vma_stop(priv, vma); if (priv->task) put_task_struct(priv->task); diff --git a/fs/seq_file.c b/fs/seq_file.c index 05d6b0e..e17d5e6 100644 --- a/fs/seq_file.c +++ b/fs/seq_file.c @@ -83,7 +83,7 @@ static int traverse(struct seq_file *m, loff_t offset) p = m->op->start(m, &index); while (p) { error = PTR_ERR(p); - if (IS_ERR(p)) + if (IS_ERR_OR_NULL(p)) break; error = m->op->show(m, p); if (error < 0) ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Patch V2] proc: check error pointer returned by m_start() 2011-03-28 5:26 ` [Patch V2] proc: check error pointer returned by m_start() Amerigo Wang @ 2011-03-28 5:45 ` Cong Wang 2011-03-28 5:46 ` Anca Emanuel 1 sibling, 0 replies; 4+ messages in thread From: Cong Wang @ 2011-03-28 5:45 UTC (permalink / raw) To: Amerigo Wang Cc: linux-kernel, WANG Cong, Linus Torvalds, Al Viro, Andrew Morton, Eric B Munson, David Rientjes, Dave Hansen, Mel Gorman, linux-fsdevel 于 2011年03月28日 13:26, Amerigo Wang 写道: > From: WANG Cong<xiyou.wangcong@gmail.com> > > V2: move the check into m_stop() as suggested by Linus, > also, most ->show() implementations assume the second parameter 'v' > is not NULL, this fixes them too. Linus, forget this one, I saw you already checked in your own fix, I will make one based on that. :) Thanks. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch V2] proc: check error pointer returned by m_start() 2011-03-28 5:26 ` [Patch V2] proc: check error pointer returned by m_start() Amerigo Wang 2011-03-28 5:45 ` Cong Wang @ 2011-03-28 5:46 ` Anca Emanuel 2011-03-28 5:49 ` Cong Wang 1 sibling, 1 reply; 4+ messages in thread From: Anca Emanuel @ 2011-03-28 5:46 UTC (permalink / raw) To: Amerigo Wang Cc: linux-kernel, WANG Cong, Linus Torvalds, Al Viro, Andrew Morton, Eric B Munson, David Rientjes, Dave Hansen, Mel Gorman, linux-fsdevel On Mon, Mar 28, 2011 at 8:26 AM, Amerigo Wang <amwang@redhat.com> wrote: > From: WANG Cong <xiyou.wangcong@gmail.com> > > V2: move the check into m_stop() as suggested by Linus, > also, most ->show() implementations assume the second parameter 'v' > is not NULL, this fixes them too. > > Anca reported a bug: > > [15117.080119] BUG: unable to handle kernel paging request at fffffffffffffff3 > [15117.080152] IP: [<ffffffff811b4989>] vma_stop+0x19/0x40 > > Linus did the initial analysis, and found this was caused > by commit ec6fd8a4355c ("report errors in /proc/*/*map* > sanely"), which replaces NULL with various ERR_PTR() cases. > > This is true, that commit changed the return value of m_start(), > which will return an error pointer on failure, but Al forgot > to check the error pointer in m_stop() which will be called > when m_start() fails. This patches fixes it. > > Reported-by: Anca Emanuel <anca.emanuel@gmail.com> > Tested-by: Anca Emanuel <anca.emanuel@gmail.com> > Cc: Linus Torvalds <torvalds@linux-foundation.org> > Cc: Al Viro <viro@zeniv.linux.org.uk> > Signed-off-by: WANG Cong <amwang@redhat.com> > Signed-off-by: WANG Cong <xiyou.wangcong@gmail.com> > > --- > > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c > index 7c708a4..8e59169 100644 > --- a/fs/proc/task_mmu.c > +++ b/fs/proc/task_mmu.c > @@ -124,8 +124,10 @@ static void *m_start(struct seq_file *m, loff_t *pos) > return ERR_PTR(-ESRCH); > > mm = mm_for_maps(priv->task); > - if (!mm || IS_ERR(mm)) > + if (IS_ERR_OR_NULL(mm)) { > + put_task_struct(priv->task); > return mm; > + } > down_read(&mm->mmap_sem); > > tail_vma = get_gate_vma(priv->task->mm); > @@ -182,6 +184,8 @@ static void m_stop(struct seq_file *m, void *v) > struct proc_maps_private *priv = m->private; > struct vm_area_struct *vma = v; > > + if (IS_ERR_OR_NULL(v)) > + return; Note: this is not functional equivalent with the previous patch. > vma_stop(priv, vma); > if (priv->task) > put_task_struct(priv->task); > diff --git a/fs/seq_file.c b/fs/seq_file.c > index 05d6b0e..e17d5e6 100644 > --- a/fs/seq_file.c > +++ b/fs/seq_file.c > @@ -83,7 +83,7 @@ static int traverse(struct seq_file *m, loff_t offset) > p = m->op->start(m, &index); > while (p) { > error = PTR_ERR(p); > - if (IS_ERR(p)) > + if (IS_ERR_OR_NULL(p)) > break; > error = m->op->show(m, p); > if (error < 0) > -- I din't test the above patch. Linus already have the fix in his tree. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch V2] proc: check error pointer returned by m_start() 2011-03-28 5:46 ` Anca Emanuel @ 2011-03-28 5:49 ` Cong Wang 0 siblings, 0 replies; 4+ messages in thread From: Cong Wang @ 2011-03-28 5:49 UTC (permalink / raw) To: Anca Emanuel Cc: linux-kernel, WANG Cong, Linus Torvalds, Al Viro, Andrew Morton, Eric B Munson, David Rientjes, Dave Hansen, Mel Gorman, linux-fsdevel 于 2011年03月28日 13:46, Anca Emanuel 写道: > On Mon, Mar 28, 2011 at 8:26 AM, Amerigo Wang<amwang@redhat.com> wrote: >> From: WANG Cong<xiyou.wangcong@gmail.com> >> >> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c >> index 7c708a4..8e59169 100644 >> --- a/fs/proc/task_mmu.c >> +++ b/fs/proc/task_mmu.c >> @@ -124,8 +124,10 @@ static void *m_start(struct seq_file *m, loff_t *pos) >> return ERR_PTR(-ESRCH); >> >> mm = mm_for_maps(priv->task); >> - if (!mm || IS_ERR(mm)) >> + if (IS_ERR_OR_NULL(mm)) { >> + put_task_struct(priv->task); >> return mm; >> + } >> down_read(&mm->mmap_sem); >> >> tail_vma = get_gate_vma(priv->task->mm); >> @@ -182,6 +184,8 @@ static void m_stop(struct seq_file *m, void *v) >> struct proc_maps_private *priv = m->private; >> struct vm_area_struct *vma = v; >> >> + if (IS_ERR_OR_NULL(v)) >> + return; > > Note: this is not functional equivalent with the previous patch. > I moved that put_task_struct() into m_start() itself. > > I din't test the above patch. > > Linus already have the fix in his tree. Yes, I really should pull before I made a patch. :-/ Anyway, thanks for reporting and testing. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-03-28 5:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <AANLkTi=mqKF=xeMYbj9LZgPfSLHSkO1JRXsNTW3LyHqu@mail.gmail.com>
2011-03-28 5:26 ` [Patch V2] proc: check error pointer returned by m_start() Amerigo Wang
2011-03-28 5:45 ` Cong Wang
2011-03-28 5:46 ` Anca Emanuel
2011-03-28 5:49 ` Cong Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).