aboutsummaryrefslogtreecommitdiffstats
path: root/fs/cifs/file.c
diff options
context:
space:
mode:
authorJeff Layton <jlayton@redhat.com>2007-11-20 18:19:03 -0500
committerSteve French <sfrench@us.ibm.com>2007-11-20 18:19:03 -0500
commitcea218054ad277d6c126890213afde07b4eb1602 (patch)
tree8bfbd3c7d8ab94d35ec749ed4e0d66b1f6b69101 /fs/cifs/file.c
parent2a97468024fb5b6eccee2a67a7796485c829343a (diff)
[CIFS] Fix potential data corruption when writing out cached dirty pages
Fix RedHat bug 329431 The idea here is separate "conscious" from "unconscious" flushes. Conscious flushes are those due to a fsync() or close(). Unconscious ones are flushes that occur as a side effect of some other operation or due to memory pressure. Currently, when an error occurs during an unconscious flush (ENOSPC or EIO), we toss out the page and don't preserve that error to report to the user when a conscious flush occurs. If after the unconscious flush, there are no more dirty pages for the inode, the conscious flush will simply return success even though there were previous errors when writing out pages. This can lead to data corruption. The easiest way to reproduce this is to mount up a CIFS share that's very close to being full or where the user is very close to quota. mv a file to the share that's slightly larger than the quota allows. The writes will all succeed (since they go to pagecache). The mv will do a setattr to set the new file's attributes. This calls filemap_write_and_wait, which will return an error since all of the pages can't be written out. Then later, when the flush and release ops occur, there are no more dirty pages in pagecache for the file and those operations return 0. mv then assumes that the file was written out correctly and deletes the original. CIFS already has a write_behind_rc variable where it stores the results from earlier flushes, but that value is only reported in cifs_close. Since the VFS ignores the return value from the release operation, this isn't helpful. We should be reporting this error during the flush operation. This patch does the following: 1) changes cifs_fsync to use filemap_write_and_wait and cifs_flush and also sync to check its return code. If it returns successful, they then check the value of write_behind_rc to see if an earlier flush had reported any errors. If so, they return that error and clear write_behind_rc. 2) sets write_behind_rc in a few other places where pages are written out as a side effect of other operations and the code waits on them. 3) changes cifs_setattr to only call filemap_write_and_wait for ATTR_SIZE changes. 4) makes cifs_writepages accurately distinguish between EIO and ENOSPC errors when writing out pages. Some simple testing indicates that the patch works as expected and that it fixes the reproduceable known problem. Acked-by: Dave Kleikamp <shaggy@austin.rr.com> Signed-off-by: Jeff Layton <jlayton@redhat.com> Signed-off-by: Steve French <sfrench@us.ibm.com>
Diffstat (limited to 'fs/cifs/file.c')
-rw-r--r--fs/cifs/file.c24
1 files changed, 18 insertions, 6 deletions
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 802564196510..dd26e2759b17 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -130,7 +130,9 @@ static inline int cifs_open_inode_helper(struct inode *inode, struct file *file,
130 if (file->f_path.dentry->d_inode->i_mapping) { 130 if (file->f_path.dentry->d_inode->i_mapping) {
131 /* BB no need to lock inode until after invalidate 131 /* BB no need to lock inode until after invalidate
132 since namei code should already have it locked? */ 132 since namei code should already have it locked? */
133 filemap_write_and_wait(file->f_path.dentry->d_inode->i_mapping); 133 rc = filemap_write_and_wait(file->f_path.dentry->d_inode->i_mapping);
134 if (rc != 0)
135 CIFS_I(file->f_path.dentry->d_inode)->write_behind_rc = rc;
134 } 136 }
135 cFYI(1, ("invalidating remote inode since open detected it " 137 cFYI(1, ("invalidating remote inode since open detected it "
136 "changed")); 138 "changed"));
@@ -425,7 +427,9 @@ reopen_error_exit:
425 pCifsInode = CIFS_I(inode); 427 pCifsInode = CIFS_I(inode);
426 if (pCifsInode) { 428 if (pCifsInode) {
427 if (can_flush) { 429 if (can_flush) {
428 filemap_write_and_wait(inode->i_mapping); 430 rc = filemap_write_and_wait(inode->i_mapping);
431 if (rc != 0)
432 CIFS_I(inode)->write_behind_rc = rc;
429 /* temporarily disable caching while we 433 /* temporarily disable caching while we
430 go to server to get inode info */ 434 go to server to get inode info */
431 pCifsInode->clientCanCacheAll = FALSE; 435 pCifsInode->clientCanCacheAll = FALSE;
@@ -1367,7 +1371,10 @@ retry:
1367 rc, bytes_written)); 1371 rc, bytes_written));
1368 /* BB what if continued retry is 1372 /* BB what if continued retry is
1369 requested via mount flags? */ 1373 requested via mount flags? */
1370 set_bit(AS_EIO, &mapping->flags); 1374 if (rc == -ENOSPC)
1375 set_bit(AS_ENOSPC, &mapping->flags);
1376 else
1377 set_bit(AS_EIO, &mapping->flags);
1371 } else { 1378 } else {
1372 cifs_stats_bytes_written(cifs_sb->tcon, 1379 cifs_stats_bytes_written(cifs_sb->tcon,
1373 bytes_written); 1380 bytes_written);
@@ -1499,9 +1506,11 @@ int cifs_fsync(struct file *file, struct dentry *dentry, int datasync)
1499 cFYI(1, ("Sync file - name: %s datasync: 0x%x", 1506 cFYI(1, ("Sync file - name: %s datasync: 0x%x",
1500 dentry->d_name.name, datasync)); 1507 dentry->d_name.name, datasync));
1501 1508
1502 rc = filemap_fdatawrite(inode->i_mapping); 1509 rc = filemap_write_and_wait(inode->i_mapping);
1503 if (rc == 0) 1510 if (rc == 0) {
1511 rc = CIFS_I(inode)->write_behind_rc;
1504 CIFS_I(inode)->write_behind_rc = 0; 1512 CIFS_I(inode)->write_behind_rc = 0;
1513 }
1505 FreeXid(xid); 1514 FreeXid(xid);
1506 return rc; 1515 return rc;
1507} 1516}
@@ -1553,8 +1562,11 @@ int cifs_flush(struct file *file, fl_owner_t id)
1553 filemapfdatawrite appears easier for the time being */ 1562 filemapfdatawrite appears easier for the time being */
1554 1563
1555 rc = filemap_fdatawrite(inode->i_mapping); 1564 rc = filemap_fdatawrite(inode->i_mapping);
1556 if (!rc) /* reset wb rc if we were able to write out dirty pages */ 1565 /* reset wb rc if we were able to write out dirty pages */
1566 if (!rc) {
1567 rc = CIFS_I(inode)->write_behind_rc;
1557 CIFS_I(inode)->write_behind_rc = 0; 1568 CIFS_I(inode)->write_behind_rc = 0;
1569 }
1558 1570
1559 cFYI(1, ("Flush inode %p file %p rc %d", inode, file, rc)); 1571 cFYI(1, ("Flush inode %p file %p rc %d", inode, file, rc));
1560 1572