From mboxrd@z Thu Jan 1 00:00:00 1970 Date: Sun, 18 Jun 2006 11:41:57 +0200 From: Nick Piggin Subject: [patch] rfc: fix splice mapping race? Message-ID: <20060618094157.GD14452@wotan.suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Sender: owner-linux-mm@kvack.org Return-Path: To: Linux Kernel Mailing List , Andrew Morton , Christoph Lameter , Jens Axboe , Linux Memory Management List List-ID: Hi, I would be interested in confirmation/comments for this patch. I believe splice is unsafe to access the page mapping obtained when the page was unlocked: the page could subsequently be truncated and the mapping reclaimed (see set_page_dirty_lock comments). Modify the remove_mapping precondition to ensure the caller has locked the page and obtained the correct mapping. Modify callers to ensure the mapping is the correct one. In page migration, detect the missing mapping early and bail out if that is the case: the page is not going to get un-truncated, so retrying is just a waste of time. Signed-off-by: Nick Piggin Index: linux-2.6/fs/splice.c =================================================================== --- linux-2.6.orig/fs/splice.c +++ linux-2.6/fs/splice.c @@ -55,9 +55,12 @@ static int page_cache_pipe_buf_steal(str struct pipe_buffer *buf) { struct page *page = buf->page; - struct address_space *mapping = page_mapping(page); + struct address_space *mapping; lock_page(page); + mapping = page_mapping(page); + if (!mapping) + goto out_failed; WARN_ON(!PageUptodate(page)); @@ -74,6 +77,7 @@ static int page_cache_pipe_buf_steal(str try_to_release_page(page, mapping_gfp_mask(mapping)); if (!remove_mapping(mapping, page)) { +out_failed: unlock_page(page); return 1; } Index: linux-2.6/mm/migrate.c =================================================================== --- linux-2.6.orig/mm/migrate.c +++ linux-2.6/mm/migrate.c @@ -136,9 +136,13 @@ static int swap_page(struct page *page) { struct address_space *mapping = page_mapping(page); - if (page_mapped(page) && mapping) + if (!mapping) + return -EINVAL; /* page truncated. signal permanent failure */ + + if (page_mapped(page)) { if (try_to_unmap(page, 1) != SWAP_SUCCESS) goto unlock_retry; + } if (PageDirty(page)) { /* Page is dirty, try to write it out here */ Index: linux-2.6/mm/vmscan.c =================================================================== --- linux-2.6.orig/mm/vmscan.c +++ linux-2.6/mm/vmscan.c @@ -362,8 +362,8 @@ pageout_t pageout(struct page *page, str int remove_mapping(struct address_space *mapping, struct page *page) { - if (!mapping) - return 0; /* truncate got there first */ + BUG_ON(!PageLocked(page)); + BUG_ON(mapping != page->mapping); write_lock_irq(&mapping->tree_lock); @@ -532,7 +532,7 @@ static unsigned long shrink_page_list(st goto free_it; } - if (!remove_mapping(mapping, page)) + if (!mapping || !remove_mapping(mapping, page)) goto keep_locked; free_it: -- 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