ext4: Fix BUG when writing to an unitialized extent

This patch fixes a bug when writing to preallocated but uninitialized
blocks, which resulted in a BUG in fs/buffer.c saying that the buffer
is not mapped.

When writing to a file, ext4_get_block_wrap() is called with create=1 in
order to request that blocks be allocated if necessary.  It currently
calls ext4_get_blocks() with create=0 in order to do a lookup first.  If
the inode contains an unitialized data block, the buffer head is left
unampped, which ext4_get_blocks_wrap() returns, causing the BUG.

We fix this by checking to see if the buffer head is unmapped, and if
so, we make sure the the buffer head is mapped by calling
ext4_ext_get_blocks with create=1.

Signed-off-by: Mingming Cao <cmm@us.ibm.com>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 8a59f7b..bcf5d04 100644 (file)
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2287,9 +2287,22 @@ out:
 }
 
 /*
+ * Block allocation/map/preallocation routine for extents based files
+ *
+ *
  * Need to be called with
  * down_read(&EXT4_I(inode)->i_data_sem) if not allocating file system block
  * (ie, create is zero). Otherwise down_write(&EXT4_I(inode)->i_data_sem)
+ *
+ * return > 0, number of of blocks already mapped/allocated
+ *          if create == 0 and these are pre-allocated blocks
+ *             buffer head is unmapped
+ *          otherwise blocks are mapped
+ *
+ * return = 0, if plain look up failed (blocks have not been allocated)
+ *          buffer head is unmapped
+ *
+ * return < 0, error case.
  */
 int ext4_ext_get_blocks(handle_t *handle, struct inode *inode,
                        ext4_lblk_t iblock,

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 34f3eb6..945cbf6 100644 (file)
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -908,11 +908,38 @@ out:
  */
 #define DIO_CREDITS 25
 
+
+/*
+ *
+ *
+ * ext4_ext4 get_block() wrapper function
+ * It will do a look up first, and returns if the blocks already mapped.
+ * Otherwise it takes the write lock of the i_data_sem and allocate blocks
+ * and store the allocated blocks in the result buffer head and mark it
+ * mapped.
+ *
+ * If file type is extents based, it will call ext4_ext_get_blocks(),
+ * Otherwise, call with ext4_get_blocks_handle() to handle indirect mapping
+ * based files
+ *
+ * On success, it returns the number of blocks being mapped or allocate.
+ * if create==0 and the blocks are pre-allocated and uninitialized block,
+ * the result buffer head is unmapped. If the create ==1, it will make sure
+ * the buffer head is mapped.
+ *
+ * It returns 0 if plain look up failed (blocks have not been allocated), in
+ * that casem, buffer head is unmapped
+ *
+ * It returns the error in case of allocation failure.
+ */
 int ext4_get_blocks_wrap(handle_t *handle, struct inode *inode, sector_t block,
                        unsigned long max_blocks, struct buffer_head *bh,
                        int create, int extend_disksize)
 {
        int retval;
+
+       clear_buffer_mapped(bh);
+
        /*
         * Try to see if we can get  the block without requesting
         * for new file system block.
@@ -926,12 +953,26 @@ int ext4_get_blocks_wrap(handle_t *handle, struct inode *inode, sector_t block,
                                inode, block, max_blocks, bh, 0, 0);
        }
        up_read((&EXT4_I(inode)->i_data_sem));
-       if (!create || (retval > 0))
+
+       /* If it is only a block(s) look up */
+       if (!create)
+               return retval;
+
+       /*
+        * Returns if the blocks have already allocated
+        *
+        * Note that if blocks have been preallocated
+        * ext4_ext_get_block() returns th create = 0
+        * with buffer head unmapped.
+        */
+       if (retval > 0 && buffer_mapped(bh))
                return retval;
 
        /*
-        * We need to allocate new blocks which will result
-        * in i_data update
+        * New blocks allocate and/or writing to uninitialized extent
+        * will possibly result in updating i_data, so we take
+        * the write lock of i_data_sem, and call get_blocks()
+        * with create == 1 flag.
         */
        down_write((&EXT4_I(inode)->i_data_sem));
        /*