[patch 39/44] cifs convert to new aops

[Nick Piggin <npiggin@suse.de>]

[-- Attachment #1: fs-cifs-aops.patch --]
[-- Type: text/plain, Size: 6271 bytes --]

Convert to new aops, and fix security hole where page is set uptodate
before contents are uptodate.

Cc: sfrench@samba.org
Cc: samba-technical@lists.samba.org
Cc: Linux Filesystems <linux-fsdevel@vger.kernel.org>
Signed-off-by: Nick Piggin <npiggin@suse.de>

 fs/cifs/file.c |   89 ++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 51 insertions(+), 38 deletions(-)

Index: linux-2.6/fs/cifs/file.c
===================================================================
--- linux-2.6.orig/fs/cifs/file.c
+++ linux-2.6/fs/cifs/file.c
@@ -103,7 +103,7 @@ static inline int cifs_open_inode_helper
 
 	/* want handles we can use to read with first
 	   in the list so we do not have to walk the
-	   list to search for one in prepare_write */
+	   list to search for one in write_begin */
 	if ((file->f_flags & O_ACCMODE) == O_WRONLY) {
 		list_add_tail(&pCifsFile->flist, 
 			      &pCifsInode->openFileList);
@@ -1358,40 +1358,37 @@ static int cifs_writepage(struct page* p
 	return rc;
 }
 
-static int cifs_commit_write(struct file *file, struct page *page,
-	unsigned offset, unsigned to)
+static int cifs_write_end(struct file *file, struct address_space *mapping,
+			loff_t pos, unsigned len, unsigned copied,
+			struct page *page, void *fsdata)
 {
 	int xid;
 	int rc = 0;
-	struct inode *inode = page->mapping->host;
-	loff_t position = ((loff_t)page->index << PAGE_CACHE_SHIFT) + to;
+	struct inode *inode = mapping->host;
+	loff_t position = pos + copied;
 	char *page_data;
 
 	xid = GetXid();
-	cFYI(1, ("commit write for page %p up to position %lld for %d", 
-		 page, position, to));
+	cFYI(1, ("write end for page %p at pos %lld, copied %d",
+		 page, pos, copied));
 	spin_lock(&inode->i_lock);
 	if (position > inode->i_size) {
 		i_size_write(inode, position);
 	}
 	spin_unlock(&inode->i_lock);
+	if (!PageUptodate(page) && copied == PAGE_CACHE_SIZE)
+		SetPageUptodate(page);
+
 	if (!PageUptodate(page)) {
-		position =  ((loff_t)page->index << PAGE_CACHE_SHIFT) + offset;
-		/* can not rely on (or let) writepage write this data */
-		if (to < offset) {
-			cFYI(1, ("Illegal offsets, can not copy from %d to %d",
-				offset, to));
-			FreeXid(xid);
-			return rc;
-		}
+		unsigned long offset = pos & (PAGE_CACHE_SIZE - 1);
+
 		/* this is probably better than directly calling
 		   partialpage_write since in this function the file handle is
 		   known which we might as well	leverage */
 		/* BB check if anything else missing out of ppw
 		   such as updating last write time */
 		page_data = kmap(page);
-		rc = cifs_write(file, page_data + offset, to-offset,
-				&position);
+		rc = cifs_write(file, page_data + offset, copied, &pos);
 		if (rc > 0)
 			rc = 0;
 		/* else if (rc < 0) should we set writebehind rc? */
@@ -1399,9 +1396,12 @@ static int cifs_commit_write(struct file
 	} else {	
 		set_page_dirty(page);
 	}
-
 	FreeXid(xid);
-	return rc;
+
+	unlock_page(page);
+	page_cache_release(page);
+
+	return rc < 0 ? rc : copied;
 }
 
 int cifs_fsync(struct file *file, struct dentry *dentry, int datasync)
@@ -1928,34 +1928,47 @@ int is_size_safe_to_change(struct cifsIn
 		return 1;
 }
 
-static int cifs_prepare_write(struct file *file, struct page *page,
-	unsigned from, unsigned to)
+static int cifs_write_begin(struct file *file, struct address_space *mapping,
+			loff_t pos, unsigned len, unsigned flags,
+			struct page **pagep, void **fsdata)
 {
 	int rc = 0;
 	loff_t i_size;
 	loff_t offset;
+	pgoff_t index = pos >> PAGE_CACHE_SHIFT;
+	struct page *page;
+
+	page = __grab_cache_page(mapping, index);
+	if (!page)
+		return -ENOMEM;
+	*pagep = page;
 
-	cFYI(1, ("prepare write for page %p from %d to %d",page,from,to));
+	cFYI(1, ("write begin for page %p at pos %lld, length %d",
+		 page, pos, len));
 	if (PageUptodate(page))
 		return 0;
 
-	/* If we are writing a full page it will be up to date,
-	   no need to read from the server */
-	if ((to == PAGE_CACHE_SIZE) && (from == 0)) {
-		SetPageUptodate(page);
+	/* If we are writing a full page it will become up to date,
+	   no need to read from the server (although we may encounter a
+	   short copy, so write_end has to handle this) */
+	if (len == PAGE_CACHE_SIZE)
 		return 0;
-	}
 
-	offset = (loff_t)page->index << PAGE_CACHE_SHIFT;
-	i_size = i_size_read(page->mapping->host);
+	offset = index << PAGE_CACHE_SHIFT;
+	i_size = i_size_read(mapping->host);
+
+	if (offset >= i_size) {
+		void *kaddr;
+		unsigned from, to;
 
-	if ((offset >= i_size) ||
-	    ((from == 0) && (offset + to) >= i_size)) {
 		/*
 		 * We don't need to read data beyond the end of the file.
 		 * zero it, and set the page uptodate
 		 */
-		void *kaddr = kmap_atomic(page, KM_USER0);
+		from = pos & (PAGE_CACHE_SIZE - 1);
+		to = from + len;
+
+		kaddr = kmap_atomic(page, KM_USER0);
 
 		if (from)
 			memset(kaddr, 0, from);
@@ -1971,12 +1984,12 @@ static int cifs_prepare_write(struct fil
 		/* we could try using another file handle if there is one -
 		   but how would we lock it to prevent close of that handle
 		   racing with this read? In any case
-		   this will be written out by commit_write so is fine */
+		   this will be written out by write_end so is fine */
 	}
 
 	/* we do not need to pass errors back 
 	   e.g. if we do not have read access to the file 
-	   because cifs_commit_write will do the right thing.  -- shaggy */
+	   because cifs_write_end will do the right thing.  -- shaggy */
 
 	return 0;
 }
@@ -1986,8 +1999,8 @@ const struct address_space_operations ci
 	.readpages = cifs_readpages,
 	.writepage = cifs_writepage,
 	.writepages = cifs_writepages,
-	.prepare_write = cifs_prepare_write,
-	.commit_write = cifs_commit_write,
+	.write_begin = cifs_write_begin,
+	.write_end = cifs_write_end,
 	.set_page_dirty = __set_page_dirty_nobuffers,
 	/* .sync_page = cifs_sync_page, */
 	/* .direct_IO = */
@@ -2002,8 +2015,8 @@ const struct address_space_operations ci
 	.readpage = cifs_readpage,
 	.writepage = cifs_writepage,
 	.writepages = cifs_writepages,
-	.prepare_write = cifs_prepare_write,
-	.commit_write = cifs_commit_write,
+	.write_begin = cifs_write_begin,
+	.write_end = cifs_write_end,
 	.set_page_dirty = __set_page_dirty_nobuffers,
 	/* .sync_page = cifs_sync_page, */
 	/* .direct_IO = */

--