[PATCH] Bug in error recovery in fs/buffer.c::__block_prepare_write()

fs/buffer.c::__block_prepare_write() has broken error recovery.  It calls
the get_block() callback with "create = 1" and if that succeeds it
immediately clears buffer_new on the just allocated buffer (which has
buffer_new set).

The bug is that if an error occurs and get_block() returns != 0, we break
from this loop and go into recovery code.  This code has this comment:

/* Error case: */
/*
 * Zero out any newly allocated blocks to avoid exposing stale
 * data.  If BH_New is set, we know that the block was newly
 * allocated in the above loop.
 */

So the intent is obviously good in that it wants to clear just allocated
and hence not zeroed buffers.  However the code recognises allocated
buffers by checking for buffer_new being set.

Unfortunately __block_prepare_write() as discussed above already cleared
buffer_new on all allocated buffers thus no buffers will be cleared during
error recovery and old data will be leaked.

The simplest way I can see to fix this is to make the current recovery code
work by _not_ clearing buffer_new after calling get_block() in
__block_prepare_write().

We cannot safely allow buffer_new buffers to "leak out" of
__block_prepare_write(), thus we simply do a quick loop over the buffers
clearing buffer_new on each of them if it is set just before returning
"success" from __block_prepare_write().

Signed-off-by: Anton Altaparmakov <aia21@cantab.net>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/fs/buffer.c b/fs/buffer.c
index 12bdb27911271202174ba43abcc789ed7eefccf0..13e5938a64f6338072e06c16ad8b73e366cad024 100644 (file)
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -1926,7 +1926,6 @@ static int __block_prepare_write(struct inode *inode, struct page *page,
                        if (err)
                                break;
                        if (buffer_new(bh)) {
-                               clear_buffer_new(bh);
                                unmap_underlying_metadata(bh->b_bdev,
                                                        bh->b_blocknr);
                                if (PageUptodate(page)) {
@@ -1968,9 +1967,14 @@ static int __block_prepare_write(struct inode *inode, struct page *page,
                if (!buffer_uptodate(*wait_bh))
                        err = -EIO;
        }
-       if (!err)
-               return err;
-
+       if (!err) {
+               bh = head;
+               do {
+                       if (buffer_new(bh))
+                               clear_buffer_new(bh);
+               } while ((bh = bh->b_this_page) != head);
+               return 0;
+       }
        /* Error case: */
        /*
         * Zero out any newly allocated blocks to avoid exposing stale