Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756790Ab0BLMCI (ORCPT ); Fri, 12 Feb 2010 07:02:08 -0500 Received: from gir.skynet.ie ([193.1.99.77]:55589 "EHLO gir.skynet.ie" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756470Ab0BLMBG (ORCPT ); Fri, 12 Feb 2010 07:01:06 -0500 From: Mel Gorman To: Andrea Arcangeli Cc: Christoph Lameter , Adam Litke , Avi Kivity , David Rientjes , KOSAKI Motohiro , Mel Gorman , linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: [PATCH 10/12] mm: Check for an empty VMA list in rmap_walk_anon Date: Fri, 12 Feb 2010 12:00:57 +0000 Message-Id: <1265976059-7459-11-git-send-email-mel@csn.ul.ie> X-Mailer: git-send-email 1.6.5 In-Reply-To: <1265976059-7459-1-git-send-email-mel@csn.ul.ie> References: <1265976059-7459-1-git-send-email-mel@csn.ul.ie> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1996 Lines: 57 There appears to be a race in rmap_walk_anon() that can be triggered by using page migration under heavy load on pages that do not belong to the process doing the migration - e.g. during memory compaction. The bug triggered is a NULL pointer deference in list_for_each_entry(). I believe what is happening is that rmap_walk() is called but the process exits before the lock gets taken. rmap_walk_anon() by design is not holding the mmap_sem which would have guaranteed its existance. There is a comment explaining the locking (or lack thereof) decision but the reasoning is not very clear. This patch checks if the VMA list is empty after the lock is taken which avoids the race. It should be reviewed by people more familiar with migration to confirm this is a sufficient or if the locking decisions around rmap_walk() need to be revisited. Signed-off-by: Mel Gorman --- mm/rmap.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-) diff --git a/mm/rmap.c b/mm/rmap.c index 278cd27..b468d5f 100644 --- a/mm/rmap.c +++ b/mm/rmap.c @@ -1237,6 +1237,14 @@ static int rmap_walk_anon(struct page *page, int (*rmap_one)(struct page *, if (!anon_vma) return ret; spin_lock(&anon_vma->lock); + + /* + * While the anon_vma may still exist, there is no guarantee + * the VMAs still do. + */ + if (list_empty(&anon_vma->head)) + goto out_anon_unlock; + list_for_each_entry(vma, &anon_vma->head, anon_vma_node) { unsigned long address = vma_address(page, vma); if (address == -EFAULT) @@ -1245,6 +1253,8 @@ static int rmap_walk_anon(struct page *page, int (*rmap_one)(struct page *, if (ret != SWAP_AGAIN) break; } + +out_anon_unlock: spin_unlock(&anon_vma->lock); return ret; } -- 1.6.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/