Convert cifs to new aops. Fix security hole where page is set uptodate before contents are uptodate. Index: linux-2.6/fs/cifs/file.c =================================================================== --- linux-2.6.orig/fs/cifs/file.c +++ linux-2.6/fs/cifs/file.c @@ -104,7 +104,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); @@ -1389,18 +1389,19 @@ 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); @@ -1432,23 +1433,19 @@ static int cifs_commit_write(struct file } */ } 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? */ @@ -1456,9 +1453,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) @@ -1988,34 +1988,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); @@ -2031,12 +2044,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; } @@ -2046,8 +2059,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 = */ @@ -2062,8 +2075,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 = */