[patch] rfc: fix splice mapping race?

[Nick Piggin]

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 <npiggin@xxxxxxx>
 

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 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/