From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752254AbdKWIFg (ORCPT ); Thu, 23 Nov 2017 03:05:36 -0500 Received: from mx2.suse.de ([195.135.220.15]:51014 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751286AbdKWIFe (ORCPT ); Thu, 23 Nov 2017 03:05:34 -0500 Subject: Re: [PATCH v2] mm/cma: fix alloc_contig_range ret code/potential leak To: Mike Kravetz , linux-mm@kvack.org, linux-kernel@vger.kernel.org Cc: Joonsoo Kim , Michal Nazarewicz , Michal Hocko , Mel Gorman , Johannes Weiner , Andrew Morton , stable@vger.kernel.org References: <15cf0f39-43f9-8287-fcfe-f2502af59e8a@oracle.com> <20171122185214.25285-1-mike.kravetz@oracle.com> From: Vlastimil Babka Message-ID: Date: Thu, 23 Nov 2017 09:05:30 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <20171122185214.25285-1-mike.kravetz@oracle.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/22/2017 07:52 PM, Mike Kravetz wrote: > If the call __alloc_contig_migrate_range() in alloc_contig_range > returns -EBUSY, processing continues so that test_pages_isolated() > is called where there is a tracepoint to identify the busy pages. > However, it is possible for busy pages to become available between > the calls to these two routines. In this case, the range of pages > may be allocated. Unfortunately, the original return code (ret > == -EBUSY) is still set and returned to the caller. Therefore, > the caller believes the pages were not allocated and they are leaked. > > Update comment to indicate that allocation is still possible even if > __alloc_contig_migrate_range returns -EBUSY. Also, clear return code > in this case so that it is not accidentally used or returned to caller. > > Fixes: 8ef5849fa8a2 ("mm/cma: always check which page caused allocation failure") > Cc: > Signed-off-by: Mike Kravetz Acked-by: Vlastimil Babka > --- > mm/page_alloc.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 77e4d3c5c57b..25e81844d1aa 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -7582,11 +7582,18 @@ int alloc_contig_range(unsigned long start, unsigned long end, > > /* > * In case of -EBUSY, we'd like to know which page causes problem. > - * So, just fall through. We will check it in test_pages_isolated(). > + * So, just fall through. test_pages_isolated() has a tracepoint > + * which will report the busy page. > + * > + * It is possible that busy pages could become available before > + * the call to test_pages_isolated, and the range will actually be > + * allocated. So, if we fall through be sure to clear ret so that > + * -EBUSY is not accidentally used or returned to caller. > */ > ret = __alloc_contig_migrate_range(&cc, start, end); > if (ret && ret != -EBUSY) > goto done; > + ret =0; ^ missing space > > /* > * Pages from [start, end) are within a MAX_ORDER_NR_PAGES > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [PATCH v2] mm/cma: fix alloc_contig_range ret code/potential leak To: Mike Kravetz , linux-mm@kvack.org, linux-kernel@vger.kernel.org Cc: Joonsoo Kim , Michal Nazarewicz , Michal Hocko , Mel Gorman , Johannes Weiner , Andrew Morton , stable@vger.kernel.org References: <15cf0f39-43f9-8287-fcfe-f2502af59e8a@oracle.com> <20171122185214.25285-1-mike.kravetz@oracle.com> From: Vlastimil Babka Message-ID: Date: Thu, 23 Nov 2017 09:05:30 +0100 MIME-Version: 1.0 In-Reply-To: <20171122185214.25285-1-mike.kravetz@oracle.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: On 11/22/2017 07:52 PM, Mike Kravetz wrote: > If the call __alloc_contig_migrate_range() in alloc_contig_range > returns -EBUSY, processing continues so that test_pages_isolated() > is called where there is a tracepoint to identify the busy pages. > However, it is possible for busy pages to become available between > the calls to these two routines. In this case, the range of pages > may be allocated. Unfortunately, the original return code (ret > == -EBUSY) is still set and returned to the caller. Therefore, > the caller believes the pages were not allocated and they are leaked. > > Update comment to indicate that allocation is still possible even if > __alloc_contig_migrate_range returns -EBUSY. Also, clear return code > in this case so that it is not accidentally used or returned to caller. > > Fixes: 8ef5849fa8a2 ("mm/cma: always check which page caused allocation failure") > Cc: > Signed-off-by: Mike Kravetz Acked-by: Vlastimil Babka > --- > mm/page_alloc.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 77e4d3c5c57b..25e81844d1aa 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -7582,11 +7582,18 @@ int alloc_contig_range(unsigned long start, unsigned long end, > > /* > * In case of -EBUSY, we'd like to know which page causes problem. > - * So, just fall through. We will check it in test_pages_isolated(). > + * So, just fall through. test_pages_isolated() has a tracepoint > + * which will report the busy page. > + * > + * It is possible that busy pages could become available before > + * the call to test_pages_isolated, and the range will actually be > + * allocated. So, if we fall through be sure to clear ret so that > + * -EBUSY is not accidentally used or returned to caller. > */ > ret = __alloc_contig_migrate_range(&cc, start, end); > if (ret && ret != -EBUSY) > goto done; > + ret =0; ^ missing space > > /* > * Pages from [start, end) are within a MAX_ORDER_NR_PAGES > -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org