include-forgotten-start-block-on-fallocate-insert-range

   1 ext4: Include forgotten start block on fallocate insert range
   2
   3 From: Roman Pen <roman.penyaev@profitbricks.com>
   4
   5 While doing 'insert range' start block should be also shifted right.
   6 The bug can be easily reproduced by the following test:
   7
   8     ptr = malloc(4096);
   9     assert(ptr);
  10
  11     fd = open("./ext4.file", O_CREAT | O_TRUNC | O_RDWR, 0600);
  12     assert(fd >= 0);
  13
  14     rc = fallocate(fd, 0, 0, 8192);
  15     assert(rc == 0);
  16     for (i = 0; i < 2048; i++)
  17             *((unsigned short *)ptr + i) = 0xbeef;
  18     rc = pwrite(fd, ptr, 4096, 0);
  19     assert(rc == 4096);
  20     rc = pwrite(fd, ptr, 4096, 4096);
  21     assert(rc == 4096);
  22
  23     for (block = 2; block < 1000; block++) {
  24             rc = fallocate(fd, FALLOC_FL_INSERT_RANGE, 4096, 4096);
  25             assert(rc == 0);
  26
  27             for (i = 0; i < 2048; i++)
  28                     *((unsigned short *)ptr + i) = block;
  29
  30             rc = pwrite(fd, ptr, 4096, 4096);
  31             assert(rc == 4096);
  32     }
  33
  34 Because start block is not included in the range the hole appears at
  35 the wrong offset (just after the desired offset) and the following
  36 pwrite() overwrites already existent block, keeping hole untouched.
  37
  38 Simple way to verify wrong behaviour is to check zeroed blocks after
  39 the test:
  40
  41    $ hexdump ./ext4.file | grep '0000 0000'
  42
  43 The root cause of the bug is a wrong range (start, stop], where start
  44 should be inclusive, i.e. [start, stop].
  45
  46 This patch fixes the problem by including start into the range.  But
  47 not to break left shift (range collapse) stop points to the beginning
  48 of the a block, not to the end.
  49
  50 The other not obvious change is an iterator check on validness in a
  51 main loop.  Because iterator is unsigned the following corner case
  52 should be considered with care: insert a block at 0 offset, when stop
  53 variables overflows and never becomes less than start, which is 0.
  54 To handle this special case iterator is set to NULL to indicate that
  55 end of the loop is reached.
  56
  57 Fixes: 331573febb6a2
  58 Signed-off-by: Roman Pen <roman.penyaev@profitbricks.com>
  59 Signed-off-by: Theodore Ts'o <tytso@mit.edu>
  60 Cc: Namjae Jeon <namjae.jeon@samsung.com>
  61 Cc: Andreas Dilger <adilger.kernel@dilger.ca>
  62 Cc: stable@vger.kernel.org
  63 ---
  64  fs/ext4/extents.c | 18 ++++++++++++------
  65  1 file changed, 12 insertions(+), 6 deletions(-)
  66
  67 diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
  68 index 3e295d3350a9..4d3014b5a3f9 100644
  69 --- a/fs/ext4/extents.c
  70 +++ b/fs/ext4/extents.c
  71 @@ -5343,8 +5343,7 @@ ext4_ext_shift_extents(struct inode *inode, handle_t *handle,
  72         if (!extent)
  73                 goto out;
  74
  75 -       stop = le32_to_cpu(extent->ee_block) +
  76 -                       ext4_ext_get_actual_len(extent);
  77 +       stop = le32_to_cpu(extent->ee_block);
  78
  79         /*
  80          * In case of left shift, Don't start shifting extents until we make
  81 @@ -5383,8 +5382,12 @@ ext4_ext_shift_extents(struct inode *inode, handle_t *handle,
  82         else
  83                 iterator = &stop;
  84
  85 -       /* Its safe to start updating extents */
  86 -       while (start < stop) {
  87 +       /*
  88 +        * Its safe to start updating extents.  Start and stop are unsigned, so
  89 +        * in case of right shift if extent with 0 block is reached, iterator
  90 +        * becomes NULL to indicate the end of the loop.
  91 +        */
  92 +       while (iterator && start <= stop) {
  93                 path = ext4_find_extent(inode, *iterator, &path, 0);
  94                 if (IS_ERR(path))
  95                         return PTR_ERR(path);
  96 @@ -5412,8 +5415,11 @@ ext4_ext_shift_extents(struct inode *inode, handle_t *handle,
  97                                         ext4_ext_get_actual_len(extent);
  98                 } else {
  99                         extent = EXT_FIRST_EXTENT(path[depth].p_hdr);
 100 -                       *iterator =  le32_to_cpu(extent->ee_block) > 0 ?
 101 -                               le32_to_cpu(extent->ee_block) - 1 : 0;
 102 +                       if (le32_to_cpu(extent->ee_block) > 0)
 103 +                               *iterator = le32_to_cpu(extent->ee_block) - 1;
 104 +                       else
 105 +                               /* Beginning is reached, end of the loop */
 106 +                               iterator = NULL;
 107                         /* Update path extent in case we need to stop */
 108                         while (le32_to_cpu(extent->ee_block) < start)
 109                                 extent++;