ext4: Fix wrong comparisons in mext_check_arguments()
authorAkira Fujita <a-fujita@rs.jp.nec.com>
Wed, 16 Sep 2009 18:28:22 +0000 (14:28 -0400)
committerTheodore Ts'o <tytso@mit.edu>
Wed, 16 Sep 2009 18:28:22 +0000 (14:28 -0400)
The mext_check_arguments() function in move_extents.c has wrong
comparisons.  orig_start which is passed from user-space is block
unit, but i_size of inode is byte unit, therefore the checks do not
work fine.  This mis-check leads to the overflow of 'len' and then
hits BUG_ON() in ext4_move_extents().  The patch fixes this issue.

Signed-off-by: Akira Fujita <a-fujita@rs.jp.nec.com>
Reviewed-by: Greg Freemyer <greg.freemyer@gmail.com>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

fs/ext4/move_extent.c

index 5821e0b..c593eb2 100644 (file)
@@ -898,6 +898,10 @@ mext_check_arguments(struct inode *orig_inode,
                          struct inode *donor_inode, __u64 orig_start,
                          __u64 donor_start, __u64 *len, __u64 moved_len)
 {
+       ext4_lblk_t orig_blocks, donor_blocks;
+       unsigned int blkbits = orig_inode->i_blkbits;
+       unsigned int blocksize = 1 << blkbits;
+
        /* Regular file check */
        if (!S_ISREG(orig_inode->i_mode) || !S_ISREG(donor_inode->i_mode)) {
                ext4_debug("ext4 move extent: The argument files should be "
@@ -972,43 +976,47 @@ mext_check_arguments(struct inode *orig_inode,
        }
 
        if (orig_inode->i_size > donor_inode->i_size) {
-               if (orig_start >= donor_inode->i_size) {
+               donor_blocks = (donor_inode->i_size + blocksize - 1) >> blkbits;
+               /* TODO: eliminate this artificial restriction */
+               if (orig_start >= donor_blocks) {
                        ext4_debug("ext4 move extent: orig start offset "
-                       "[%llu] should be less than donor file size "
-                       "[%lld] [ino:orig %lu, donor_inode %lu]\n",
-                       orig_start, donor_inode->i_size,
+                       "[%llu] should be less than donor file blocks "
+                       "[%u] [ino:orig %lu, donor %lu]\n",
+                       orig_start, donor_blocks,
                        orig_inode->i_ino, donor_inode->i_ino);
                        return -EINVAL;
                }
 
-               if (orig_start + *len > donor_inode->i_size) {
+               /* TODO: eliminate this artificial restriction */
+               if (orig_start + *len > donor_blocks) {
                        ext4_debug("ext4 move extent: End offset [%llu] should "
-                               "be less than donor file size [%lld]."
-                               "So adjust length from %llu to %lld "
+                               "be less than donor file blocks [%u]."
+                               "So adjust length from %llu to %llu "
                                "[ino:orig %lu, donor %lu]\n",
-                               orig_start + *len, donor_inode->i_size,
-                               *len, donor_inode->i_size - orig_start,
+                               orig_start + *len, donor_blocks,
+                               *len, donor_blocks - orig_start,
                                orig_inode->i_ino, donor_inode->i_ino);
-                       *len = donor_inode->i_size - orig_start;
+                       *len = donor_blocks - orig_start;
                }
        } else {
-               if (orig_start >= orig_inode->i_size) {
+               orig_blocks = (orig_inode->i_size + blocksize - 1) >> blkbits;
+               if (orig_start >= orig_blocks) {
                        ext4_debug("ext4 move extent: start offset [%llu] "
-                               "should be less than original file size "
-                               "[%lld] [inode:orig %lu, donor %lu]\n",
-                                orig_start, orig_inode->i_size,
+                               "should be less than original file blocks "
+                               "[%u] [ino:orig %lu, donor %lu]\n",
+                                orig_start, orig_blocks,
                                orig_inode->i_ino, donor_inode->i_ino);
                        return -EINVAL;
                }
 
-               if (orig_start + *len > orig_inode->i_size) {
+               if (orig_start + *len > orig_blocks) {
                        ext4_debug("ext4 move extent: Adjust length "
-                               "from %llu to %lld. Because it should be "
-                               "less than original file size "
+                               "from %llu to %llu. Because it should be "
+                               "less than original file blocks "
                                "[ino:orig %lu, donor %lu]\n",
-                               *len, orig_inode->i_size - orig_start,
+                               *len, orig_blocks - orig_start,
                                orig_inode->i_ino, donor_inode->i_ino);
-                       *len = orig_inode->i_size - orig_start;
+                       *len = orig_blocks - orig_start;
                }
        }