The attached patch for filemap.c fixes a problem in generic_file_write
that was causing corruption in certain NFS writes. This was occurring
because pages beyond the current end of file were not being cleared
before being marked up-to-date for a partial write. If the pattern of
writes then left gaps in the file, these would typically contain garbage
from the prior use of the page. (Maybe a security problem as well --
writing bytes sparsely would let you see various pages.)
I've also added a comment about the current practice of setting
PG_uptodate before actually copying the data into the page. I don't
think this is right, but it will require some further analysis because
of the complicated paths in nfs_updatepage. (And to see what other fs
are doing in updatepage.)
Testing the patch using gcc build with make bootstrap and make compare
is now clean.
The patch also includes a fix for a bug in get_cached_page() that was
causing oopses in smbfs -- the page_address() function was being called
for a NULL page, instead of returning 0. I posted this before, but it
hasn't appeared in the pre-80 files so I'm leaving it in this patch.
Regards,
Bill
--------------6ADB5CF9D217259F849692D6
Content-Type: text/plain; charset=us-ascii; name="filemap_79-patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="filemap_79-patch"
--- mm/filemap.c.old Tue Jan 13 10:38:36 1998
+++ mm/filemap.c Sun Jan 18 12:22:15 1998
@@ -1349,11 +1349,10 @@
if (!(page = __find_page(inode, pgpos, *hash))) {
if (!page_cache) {
page_cache = __get_free_page(GFP_KERNEL);
- if (!page_cache) {
- status = -ENOMEM;
- break;
- }
- continue;
+ if (page_cache)
+ continue;
+ status = -ENOMEM;
+ break;
}
page = mem_map + MAP_NR(page_cache);
add_to_page_cache(page, inode, pgpos, hash);
@@ -1361,34 +1360,45 @@
}
/*
- * WSH 06/05/97: restructured slightly to make sure we release
- * the page on an error exit. Removed explicit setting of
- * PG_locked, as that's handled below the i_op->xxx interface.
+ * Note: setting of the PG_locked bit is handled
+ * below the i_op->xxx interface.
*/
didread = 0;
page_wait:
wait_on_page(page);
+ if (PageUptodate(page))
+ goto do_update_page;
/*
- * If the page is not uptodate, and we're writing less
+ * The page is not up-to-date ... if we're writing less
* than a full page of data, we may have to read it first.
- * However, don't bother with reading the page when it's
- * after the current end of file.
+ * But if the page is past the current end of file, we must
+ * clear it before updating.
*/
- if (!PageUptodate(page)) {
- if (bytes < PAGE_SIZE && pgpos < inode->i_size) {
- status = -EIO; /* two tries ... error out */
- if (didread < 2)
- status = inode->i_op->readpage(dentry,
- page);
+ if (bytes < PAGE_SIZE) {
+ if (pgpos < inode->i_size) {
+ status = -EIO;
+ if (didread >= 2)
+ goto done_with_page;
+ status = inode->i_op->readpage(dentry, page);
if (status < 0)
goto done_with_page;
didread++;
goto page_wait;
+ } else {
+ /* Must clear for partial writes */
+ memset((void *) page_address(page), 0,
+ PAGE_SIZE);
}
- set_bit(PG_uptodate, &page->flags);
}
+ /*
+ * N.B. We should defer setting PG_uptodate at least until
+ * the data is copied. A failure in i_op->updatepage() could
+ * leave the page with garbage data.
+ */
+ set_bit(PG_uptodate, &page->flags);
+do_update_page:
/* Alright, the page is there. Now update it. */
status = inode->i_op->updatepage(dentry, page, buf,
offset, bytes, sync);
@@ -1408,9 +1418,7 @@
if (page_cache)
free_page(page_cache);
- if (written)
- return written;
- return status;
+ return written ? written : status;
}
/*
@@ -1429,7 +1437,7 @@
{
struct page * page;
struct page ** hash;
- unsigned long page_cache;
+ unsigned long page_cache = 0;
hash = page_hash(inode, offset);
page = __find_page(inode, offset, *hash);
@@ -1443,14 +1451,15 @@
add_to_page_cache(page, inode, offset, hash);
}
if (atomic_read(&page->count) != 2)
- printk("get_cached_page: page count=%d\n",
+ printk(KERN_ERR "get_cached_page: page count=%d\n",
atomic_read(&page->count));
if (test_bit(PG_locked, &page->flags))
- printk("get_cached_page: page already locked!\n");
+ printk(KERN_ERR "get_cached_page: page already locked!\n");
set_bit(PG_locked, &page->flags);
+ page_cache = page_address(page);
out:
- return page_address(page);
+ return page_cache;
}
/*
--------------6ADB5CF9D217259F849692D6--