nilfs2: fix kernel oops in error case of nilfs_ioctl_move_blocks
authorRyusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
Sat, 7 Nov 2009 09:45:16 +0000 (18:45 +0900)
committerRyusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
Sun, 8 Nov 2009 10:01:35 +0000 (19:01 +0900)
This fixes a kernel oops reported by Markus Trippelsdorf in the email
titled "[NILFS users] kernel Oops while running nilfs_cleanerd".

The oops was caused by a bug of error path in
nilfs_ioctl_move_blocks() function, which was inlined in
nilfs_ioctl_clean_segments().

nilfs_ioctl_move_blocks checks duplication of blocks which will be
moved in garbage collection.  But, the check should have be done
within nilfs_ioctl_move_inode_block() to prevent list corruption among
buffers storing the target blocks.

To fix the kernel oops, this moves forward the duplication check
before the list insertion.

I also tested this for stable trees [2.6.30, 2.6.31].

Reported-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Signed-off-by: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
Cc: stable <stable@kernel.org>
fs/nilfs2/ioctl.c

index 6572ea4bc4df70498dc62b29b93968d0396c7775..89dd73ead9ac42cf33f0cc72e90a4f2b03ecd447 100644 (file)
@@ -297,7 +297,18 @@ static int nilfs_ioctl_move_inode_block(struct inode *inode,
                               (unsigned long long)vdesc->vd_vblocknr);
                return ret;
        }
-       bh->b_private = vdesc;
+       if (unlikely(!list_empty(&bh->b_assoc_buffers))) {
+               printk(KERN_CRIT "%s: conflicting %s buffer: ino=%llu, "
+                      "cno=%llu, offset=%llu, blocknr=%llu, vblocknr=%llu\n",
+                      __func__, vdesc->vd_flags ? "node" : "data",
+                      (unsigned long long)vdesc->vd_ino,
+                      (unsigned long long)vdesc->vd_cno,
+                      (unsigned long long)vdesc->vd_offset,
+                      (unsigned long long)vdesc->vd_blocknr,
+                      (unsigned long long)vdesc->vd_vblocknr);
+               brelse(bh);
+               return -EEXIST;
+       }
        list_add_tail(&bh->b_assoc_buffers, buffers);
        return 0;
 }
@@ -335,24 +346,10 @@ static int nilfs_ioctl_move_blocks(struct the_nilfs *nilfs,
        list_for_each_entry_safe(bh, n, &buffers, b_assoc_buffers) {
                ret = nilfs_gccache_wait_and_mark_dirty(bh);
                if (unlikely(ret < 0)) {
-                       if (ret == -EEXIST) {
-                               vdesc = bh->b_private;
-                               printk(KERN_CRIT
-                                      "%s: conflicting %s buffer: "
-                                      "ino=%llu, cno=%llu, offset=%llu, "
-                                      "blocknr=%llu, vblocknr=%llu\n",
-                                      __func__,
-                                      vdesc->vd_flags ? "node" : "data",
-                                      (unsigned long long)vdesc->vd_ino,
-                                      (unsigned long long)vdesc->vd_cno,
-                                      (unsigned long long)vdesc->vd_offset,
-                                      (unsigned long long)vdesc->vd_blocknr,
-                                      (unsigned long long)vdesc->vd_vblocknr);
-                       }
+                       WARN_ON(ret == -EEXIST);
                        goto failed;
                }
                list_del_init(&bh->b_assoc_buffers);
-               bh->b_private = NULL;
                brelse(bh);
        }
        return nmembs;
@@ -360,7 +357,6 @@ static int nilfs_ioctl_move_blocks(struct the_nilfs *nilfs,
  failed:
        list_for_each_entry_safe(bh, n, &buffers, b_assoc_buffers) {
                list_del_init(&bh->b_assoc_buffers);
-               bh->b_private = NULL;
                brelse(bh);
        }
        return ret;