Re: [PATCH] rmap: fix anon_vma_fork() memory leak

[Minchan Kim]

On Tue, Apr 6, 2010 at 12:37 AM, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxx>
>> Signed-off-by: Rik van Riel <riel@xxxxxxxxxx>
>> Reviewed-by: Minchan Kim <minchan.kim@xxxxxxxxx>
>> ---
>>
>> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/