From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932598AbbHXLcR (ORCPT ); Mon, 24 Aug 2015 07:32:17 -0400 Received: from mail-wi0-f180.google.com ([209.85.212.180]:34720 "EHLO mail-wi0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751475AbbHXLcQ (ORCPT ); Mon, 24 Aug 2015 07:32:16 -0400 Date: Mon, 24 Aug 2015 13:32:13 +0200 From: Michal Hocko To: gang.chen.5i5j@qq.com Cc: akpm@linux-foundation.org, kirill.shutemov@linux.intel.com, riel@redhat.com, sasha.levin@oracle.com, gang.chen.5i5j@gmail.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm: mmap: Check all failures before set values Message-ID: <20150824113212.GL17078@dhcp22.suse.cz> References: <1440349179-18304-1-git-send-email-gang.chen.5i5j@qq.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1440349179-18304-1-git-send-email-gang.chen.5i5j@qq.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon 24-08-15 00:59:39, gang.chen.5i5j@qq.com wrote: > From: Chen Gang > > When failure occurs and return, vma->vm_pgoff is already set, which is > not a good idea. Why? The vma is not inserted anywhere and the failure path is supposed to simply free the vma. > Signed-off-by: Chen Gang > --- > mm/mmap.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/mm/mmap.c b/mm/mmap.c > index 8e0366e..b5a6f09 100644 > --- a/mm/mmap.c > +++ b/mm/mmap.c > @@ -2878,6 +2878,13 @@ int insert_vm_struct(struct mm_struct *mm, struct vm_area_struct *vma) > struct vm_area_struct *prev; > struct rb_node **rb_link, *rb_parent; > > + if (find_vma_links(mm, vma->vm_start, vma->vm_end, > + &prev, &rb_link, &rb_parent)) > + return -ENOMEM; > + if ((vma->vm_flags & VM_ACCOUNT) && > + security_vm_enough_memory_mm(mm, vma_pages(vma))) > + return -ENOMEM; > + > /* > * The vm_pgoff of a purely anonymous vma should be irrelevant > * until its first write fault, when page's anon_vma and index > @@ -2894,12 +2901,6 @@ int insert_vm_struct(struct mm_struct *mm, struct vm_area_struct *vma) > BUG_ON(vma->anon_vma); > vma->vm_pgoff = vma->vm_start >> PAGE_SHIFT; > } > - if (find_vma_links(mm, vma->vm_start, vma->vm_end, > - &prev, &rb_link, &rb_parent)) > - return -ENOMEM; > - if ((vma->vm_flags & VM_ACCOUNT) && > - security_vm_enough_memory_mm(mm, vma_pages(vma))) > - return -ENOMEM; > > vma_link(mm, vma, prev, rb_link, rb_parent); > return 0; > -- > 1.9.3 -- Michal Hocko SUSE Labs