[PATCH] nfs: clear_commit_release incorrectly handle truncated page

[Dmitry Monakhov <dmonakhov@openvz.org>]

[-- Attachment #1: Type: text/plain, Size: 1744 bytes --]


After page was truncated it lost it's mapping, this result in null
pointer dereference on bdi_stat update. In fact we have to decrement
bdi_stat even for truncated pages, so let's pass correct mapping in
function arguments. Patch against linux-2.6
##TEST_CASE
/*
Tast case for bug in nfs_clear_request_commit()
caused by null pointer dereference in case of truncated page.
It takes less than 10 minutes to reproduce the bug.
### start script
#! /bin/bash -x
# mount my-host:/my-share /mnt
mkdir /mnt/T
cd /mnt/T || exit 1
while true ;
      do for ((i=0;i<3;i++)); do
      	 /tmp/mmap file3 200000000 $i & done;
	 sleep 2 ;
	 killall -9 mmap ;
      done
done
*/
#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
#include <fcntl.h>
#include <sys/mman.h>
int main(int argc, char *argv[])
{
	char *addr;
	unsigned int size;
	off_t off;
	int i = 0;
	int fd, result;
	if (argc != 4) {
		perror("Wrong args:\n Usage: %s [file] [size] [offset]\n");
		exit(EXIT_FAILURE);
	}
	size = atoi(argv[2]);
	size &= ~(4096-1);
	off = atol(argv[3]);
	off = ((off_t)size)*off;
	fd = open(argv[1], O_RDWR | O_CREAT | O_TRUNC, (mode_t)0600);
	if (fd == -1) {
		perror("Error opening file for writing");
		exit(EXIT_FAILURE);
	}
	result = lseek(fd, off + (off_t)size, SEEK_SET);
	if (result == -1) {
		close(fd);
		perror("Error calling lseek() to 'stretch' the file");
		exit(EXIT_FAILURE);
	}
	write(fd, "a", 1);
	addr = mmap(0, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, off);
	if (addr == MAP_FAILED) {
		close(fd);
		perror("Error mmapping the file");
		exit(EXIT_FAILURE);
	}
	/* Now write int's to the file as if it were memory (an array of ints).
	 */
	while (1) {
		memset(addr, i++, size);
	}
}


[-- Attachment #2: 0001-nfs-clear_commit_release-incorrectly-handle-truncate.patch --]
[-- Type: text/plain, Size: 3113 bytes --]

>From bdce13e6947ad738b68bcb9fd507885d14dca9f0 Mon Sep 17 00:00:00 2001
From: Dmitry Monakhov <dmonakhov@openvz.org>
Date: Tue, 2 Feb 2010 13:24:58 +0300
Subject: [PATCH] nfs: clear_commit_release incorrectly handle truncated page

After page was truncated it lost it's mapping, this result in null
pointer dereference on bdi_stat update. In fact we have to decrement
bdi_stat even for truncated pages, so let's pass correct mapping in
function arguments.

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/nfs/write.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index d171696..bfcf92a 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -445,13 +445,13 @@ nfs_mark_request_commit(struct nfs_page *req)
 }
 
 static int
-nfs_clear_request_commit(struct nfs_page *req)
+nfs_clear_request_commit(struct nfs_page *req, struct address_space *mapping)
 {
 	struct page *page = req->wb_page;
-
+	/* page->mapping may be NULL if page was truncated */
 	if (test_and_clear_bit(PG_CLEAN, &(req)->wb_flags)) {
 		dec_zone_page_state(page, NR_UNSTABLE_NFS);
-		dec_bdi_stat(page->mapping->backing_dev_info, BDI_RECLAIMABLE);
+		dec_bdi_stat(mapping->backing_dev_info, BDI_RECLAIMABLE);
 		return 1;
 	}
 	return 0;
@@ -483,7 +483,7 @@ nfs_mark_request_commit(struct nfs_page *req)
 }
 
 static inline int
-nfs_clear_request_commit(struct nfs_page *req)
+nfs_clear_request_commit(struct nfs_page *req, struct address_space *mapping)
 {
 	return 0;
 }
@@ -539,14 +539,15 @@ static int nfs_wait_on_requests_locked(struct inode *inode, pgoff_t idx_start, u
 	return res;
 }
 
-static void nfs_cancel_commit_list(struct list_head *head)
+static void nfs_cancel_commit_list(struct list_head *head,
+				struct address_space *mapping)
 {
 	struct nfs_page *req;
 
 	while(!list_empty(head)) {
 		req = nfs_list_entry(head->next);
 		nfs_list_remove_request(req);
-		nfs_clear_request_commit(req);
+		nfs_clear_request_commit(req, mapping);
 		nfs_inode_remove_request(req);
 		nfs_unlock_request(req);
 	}
@@ -642,7 +643,7 @@ static struct nfs_page *nfs_try_to_update_request(struct inode *inode,
 		spin_lock(&inode->i_lock);
 	}
 
-	if (nfs_clear_request_commit(req))
+	if (nfs_clear_request_commit(req, inode->i_mapping))
 		radix_tree_tag_clear(&NFS_I(inode)->nfs_page_tree,
 				req->wb_index, NFS_PAGE_TAG_COMMIT);
 
@@ -1352,7 +1353,7 @@ static void nfs_commit_release(void *calldata)
 	while (!list_empty(&data->pages)) {
 		req = nfs_list_entry(data->pages.next);
 		nfs_list_remove_request(req);
-		nfs_clear_request_commit(req);
+		nfs_clear_request_commit(req, data->inode->i_mapping);
 
 		dprintk("NFS:       commit (%s/%lld %d@%lld)",
 			req->wb_context->path.dentry->d_inode->i_sb->s_id,
@@ -1449,7 +1450,7 @@ long nfs_sync_mapping_wait(struct address_space *mapping, struct writeback_contr
 			break;
 		if (how & FLUSH_INVALIDATE) {
 			spin_unlock(&inode->i_lock);
-			nfs_cancel_commit_list(&head);
+			nfs_cancel_commit_list(&head, mapping);
 			ret = pages;
 			spin_lock(&inode->i_lock);
 			continue;
-- 
1.6.3.3