rmap: fix anon_vma_fork() memory leak
in [Kernel]
|
Prev: Swap including order of util.h and string.h of util/string.c
Next: Fix key f70f4b50 not in .data in thermal_sys V2
From: Minchan Kim on 4 Apr 2010 20:00 On Mon, Apr 5, 2010 at 8:09 AM, Rik van Riel <riel(a)redhat.com> wrote: > Fix a memory leak in anon_vma_fork(), where we fail to tear down the > anon_vmas attached to the new VMA in case setting up the new anon_vma > fails. > > Reported-by: Minchan Kim <minchan.kim(a)gmail.com> > Signed-off-by: Rik van Riel <riel(a)redhat.com> Reviewed-by: Minchan Kim <minchan.kim(a)gmail.com> -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Linus Torvalds on 5 Apr 2010 11:50 On Sun, 4 Apr 2010, Rik van Riel wrote: > > Fix a memory leak in anon_vma_fork(), where we fail to tear down the > anon_vmas attached to the new VMA in case setting up the new anon_vma > fails. > > Reported-by: Minchan Kim <minchan.kim(a)gmail.com> > Signed-off-by: Rik van Riel <riel(a)redhat.com> > Reviewed-by: Minchan Kim <minchan.kim(a)gmail.com> > --- > > diff --git a/mm/rmap.c b/mm/rmap.c > index fcd593c..fb7ce99 100644 > --- a/mm/rmap.c > +++ b/mm/rmap.c > @@ -231,6 +231,7 @@ int anon_vma_fork(struct vm_area_struct *vma, struct vm_area_struct *pvma) > > out_error_free_anon_vma: > anon_vma_free(anon_vma); > + unlink_anon_vmas(vma); > out_error: > return -ENOMEM; > } This looks _very_ wrong to me. Shouldn't the unlink_anon_vmas() be in the "out_error" case? IOW, we should do it even if the "anon_vma_alloc()" failed, nbot just if the "anon_vma_chain_alloc()" failed? No? What am I missing? Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Minchan Kim on 5 Apr 2010 12:00 On Tue, Apr 6, 2010 at 12:37 AM, Linus Torvalds <torvalds(a)linux-foundation.org> wrote: > > > On Sun, 4 Apr 2010, Rik van Riel wrote: >> >> Fix a memory leak in anon_vma_fork(), where we fail to tear down the >> anon_vmas attached to the new VMA in case setting up the new anon_vma >> fails. >> >> Reported-by: Minchan Kim <minchan.kim(a)gmail.com> >> Signed-off-by: Rik van Riel <riel(a)redhat.com> >> Reviewed-by: Minchan Kim <minchan.kim(a)gmail.com> >> --- >> >> diff --git a/mm/rmap.c b/mm/rmap.c >> index fcd593c..fb7ce99 100644 >> --- a/mm/rmap.c >> +++ b/mm/rmap.c >> @@ -231,6 +231,7 @@ int anon_vma_fork(struct vm_area_struct *vma, struct vm_area_struct *pvma) >> >> out_error_free_anon_vma: >> anon_vma_free(anon_vma); >> + unlink_anon_vmas(vma); >> out_error: >> return -ENOMEM; >> } > > This looks _very_ wrong to me. > > Shouldn't the unlink_anon_vmas() be in the "out_error" case? IOW, we > should do it even if the "anon_vma_alloc()" failed, nbot just if the > "anon_vma_chain_alloc()" failed? > > No? > > What am I missing? Indeed. You're right. I should have been reviewed more carefully. -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Rik van Riel on 5 Apr 2010 12:10
On 04/05/2010 11:37 AM, Linus Torvalds wrote: > This looks _very_ wrong to me. > > Shouldn't the unlink_anon_vmas() be in the "out_error" case? Indeed it should. I've had my mind somewhere else this weekend :/ New patch in the next mail. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |