linux-toradex.git/fs/btrfs, branch v3.12.49

Btrfs: check if previous transaction aborted to avoid fs corruption

2015-09-30T09:13:08+00:00

commit 1f9b8c8fbc9a4d029760b16f477b9d15500e3a34 upstream.

While we are committing a transaction, it's possible the previous one is
still finishing its commit and therefore we wait for it to finish first.
However we were not checking if that previous transaction ended up getting
aborted after we waited for it to commit, so we ended up committing the
current transaction which can lead to fs corruption because the new
superblock can point to trees that have had one or more nodes/leafs that
were never durably persisted.
The following sequence diagram exemplifies how this is possible:

          CPU 0                                                        CPU 1

  transaction N starts

  (...)

  btrfs_commit_transaction(N)

    cur_trans->state = TRANS_STATE_COMMIT_START;
    (...)
    cur_trans->state = TRANS_STATE_COMMIT_DOING;
    (...)

    cur_trans->state = TRANS_STATE_UNBLOCKED;
    root->fs_info->running_transaction = NULL;

                                                              btrfs_start_transaction()
                                                                 --> starts transaction N + 1

    btrfs_write_and_wait_transaction(trans, root);
      --> starts writing all new or COWed ebs created
          at transaction N

                                                              creates some new ebs, COWs some
                                                              existing ebs but doesn't COW or
                                                              deletes eb X

                                                              btrfs_commit_transaction(N + 1)
                                                                (...)
                                                                cur_trans->state = TRANS_STATE_COMMIT_START;
                                                                (...)
                                                                wait_for_commit(root, prev_trans);
                                                                  --> prev_trans == transaction N

    btrfs_write_and_wait_transaction() continues
    writing ebs
       --> fails writing eb X, we abort transaction N
           and set bit BTRFS_FS_STATE_ERROR on
           fs_info->fs_state, so no new transactions
           can start after setting that bit

       cleanup_transaction()
         btrfs_cleanup_one_transaction()
           wakes up task at CPU 1

                                                                continues, doesn't abort because
                                                                cur_trans->aborted (transaction N + 1)
                                                                is zero, and no checks for bit
                                                                BTRFS_FS_STATE_ERROR in fs_info->fs_state
                                                                are made

                                                                btrfs_write_and_wait_transaction(trans, root);
                                                                  --> succeeds, no errors during writeback

                                                                write_ctree_super(trans, root, 0);
                                                                  --> succeeds
                                                                  --> we have now a superblock that points us
                                                                      to some root that uses eb X, which was
                                                                      never written to disk

In this scenario future attempts to read eb X from disk results in an
error message like "parent transid verify failed on X wanted Y found Z".

So fix this by aborting the current transaction if after waiting for the
previous transaction we verify that it was aborted.

Signed-off-by: Filipe Manana 
Reviewed-by: Josef Bacik 
Reviewed-by: Liu Bo 
Signed-off-by: Chris Mason 
Signed-off-by: Jiri Slaby

Btrfs: fix memory leak in the extent_same ioctl

2015-08-04T14:52:25+00:00

commit 497b4050e0eacd4c746dd396d14916b1e669849d upstream.

We were allocating memory with memdup_user() but we were never releasing
that memory. This affected pretty much every call to the ioctl, whether
it deduplicated extents or not.

This issue was reported on IRC by Julian Taylor and on the mailing list
by Marcel Ritter, credit goes to them for finding the issue.

Reported-by: Julian Taylor 
Reported-by: Marcel Ritter 
Signed-off-by: Filipe Manana 
Reviewed-by: Mark Fasheh 
Signed-off-by: Jiri Slaby

Btrfs: use kmem_cache_free when freeing entry in inode cache

2015-08-04T14:52:25+00:00

commit c3f4a1685bb87e59c886ee68f7967eae07d4dffa upstream.

The free space entries are allocated using kmem_cache_zalloc(),
through __btrfs_add_free_space(), therefore we should use
kmem_cache_free() and not kfree() to avoid any confusion and
any potential problem. Looking at the kfree() definition at
mm/slab.c it has the following comment:

  /*
   * (...)
   *
   * Don't free memory not originally allocated by kmalloc()
   * or you will run into trouble.
   */

So better be safe and use kmem_cache_free().

Signed-off-by: Filipe Manana 
Reviewed-by: David Sterba 
Signed-off-by: Chris Mason 
Signed-off-by: Jiri Slaby

Btrfs: make xattr replace operations atomic

2015-07-30T11:21:24+00:00

commit 5f5bc6b1e2d5a6f827bc860ef2dc5b6f365d1339 upstream.

Replacing a xattr consists of doing a lookup for its existing value, delete
the current value from the respective leaf, release the search path and then
finally insert the new value. This leaves a time window where readers (getxattr,
listxattrs) won't see any value for the xattr. Xattrs are used to store ACLs,
so this has security implications.

This change also fixes 2 other existing issues which were:

*) Deleting the old xattr value without verifying first if the new xattr will
   fit in the existing leaf item (in case multiple xattrs are packed in the
   same item due to name hash collision);

*) Returning -EEXIST when the flag XATTR_CREATE is given and the xattr doesn't
   exist but we have have an existing item that packs muliple xattrs with
   the same name hash as the input xattr. In this case we should return ENOSPC.

A test case for xfstests follows soon.

Thanks to Alexandre Oliva for reporting the non-atomicity of the xattr replace
implementation.

Reported-by: Alexandre Oliva 
Signed-off-by: Filipe Manana 
Signed-off-by: Chris Mason 
Signed-off-by: Jiri Slaby

btrfs: cleanup orphans while looking up default subvolume

2015-06-23T12:55:14+00:00

commit 727b9784b6085c99c2f836bf4fcc2848dc9cf904 upstream.

Orphans in the fs tree are cleaned up via open_ctree and subvolume
orphans are cleaned via btrfs_lookup_dentry -- except when a default
subvolume is in use.  The name for the default subvolume uses a manual
lookup that doesn't trigger orphan cleanup and needs to trigger it
manually as well. This doesn't apply to the remount case since the
subvolumes are cleaned up by walking the root radix tree.

Signed-off-by: Jeff Mahoney 
Reviewed-by: David Sterba 
Signed-off-by: Chris Mason 
Signed-off-by: Jiri Slaby

btrfs: incorrect handling for fiemap_fill_next_extent return

2015-06-23T12:55:14+00:00

commit 26e726afe01c1c82072cf23a5ed89ce25f39d9f2 upstream.

fiemap_fill_next_extent returns 0 on success, -errno on error, 1 if this was
the last extent that will fit in user array. If 1 is returned, the return
value may eventually returned to user space, which should not happen, according
to manpage of ioctl.

Signed-off-by: Chengyu Song 
Reviewed-by: David Sterba 
Reviewed-by: Liu Bo 
Signed-off-by: Chris Mason 
Signed-off-by: Jiri Slaby

Btrfs: fix inode eviction infinite loop after extent_same ioctl

2015-04-30T11:49:27+00:00

commit 113e8283869b9855c8b999796aadd506bbac155f upstream.

If we pass a length of 0 to the extent_same ioctl, we end up locking an
extent range with a start offset greater then its end offset (if the
destination file's offset is greater than zero). This results in a warning
from extent_io.c:insert_state through the following call chain:

  btrfs_extent_same()
    btrfs_double_lock()
      lock_extent_range()
        lock_extent(inode->io_tree, offset, offset + len - 1)
          lock_extent_bits()
            __set_extent_bit()
              insert_state()
                --> WARN_ON(end < start)

This leads to an infinite loop when evicting the inode. This is the same
problem that my previous patch titled
"Btrfs: fix inode eviction infinite loop after cloning into it" addressed
but for the extent_same ioctl instead of the clone ioctl.

Signed-off-by: Filipe Manana 
Reviewed-by: Omar Sandoval 
Signed-off-by: Chris Mason 
Signed-off-by: Jiri Slaby

Btrfs: fix inode eviction infinite loop after cloning into it

2015-04-30T11:49:27+00:00

commit ccccf3d67294714af2d72a6fd6fd7d73b01c9329 upstream.

If we attempt to clone a 0 length region into a file we can end up
inserting a range in the inode's extent_io tree with a start offset
that is greater then the end offset, which triggers immediately the
following warning:

[ 3914.619057] WARNING: CPU: 17 PID: 4199 at fs/btrfs/extent_io.c:435 insert_state+0x4b/0x10b [btrfs]()
[ 3914.620886] BTRFS: end < start 4095 4096
(...)
[ 3914.638093] Call Trace:
[ 3914.638636]  [] dump_stack+0x4c/0x65
[ 3914.639620]  [] warn_slowpath_common+0xa1/0xbb
[ 3914.640789]  [] ? insert_state+0x4b/0x10b [btrfs]
[ 3914.642041]  [] warn_slowpath_fmt+0x46/0x48
[ 3914.643236]  [] insert_state+0x4b/0x10b [btrfs]
[ 3914.644441]  [] __set_extent_bit+0x107/0x3f4 [btrfs]
[ 3914.645711]  [] lock_extent_bits+0x65/0x1bf [btrfs]
[ 3914.646914]  [] ? _raw_spin_unlock+0x28/0x33
[ 3914.648058]  [] ? test_range_bit+0xcc/0xde [btrfs]
[ 3914.650105]  [] lock_extent+0x13/0x15 [btrfs]
[ 3914.651361]  [] lock_extent_range+0x3d/0xcd [btrfs]
[ 3914.652761]  [] btrfs_ioctl_clone+0x278/0x388 [btrfs]
[ 3914.654128]  [] ? might_fault+0x58/0xb5
[ 3914.655320]  [] btrfs_ioctl+0xb51/0x2195 [btrfs]
(...)
[ 3914.669271] ---[ end trace 14843d3e2e622fc1 ]---

This later makes the inode eviction handler enter an infinite loop that
keeps dumping the following warning over and over:

[ 3915.117629] WARNING: CPU: 22 PID: 4228 at fs/btrfs/extent_io.c:435 insert_state+0x4b/0x10b [btrfs]()
[ 3915.119913] BTRFS: end < start 4095 4096
(...)
[ 3915.137394] Call Trace:
[ 3915.137913]  [] dump_stack+0x4c/0x65
[ 3915.139154]  [] warn_slowpath_common+0xa1/0xbb
[ 3915.140316]  [] ? insert_state+0x4b/0x10b [btrfs]
[ 3915.141505]  [] warn_slowpath_fmt+0x46/0x48
[ 3915.142709]  [] insert_state+0x4b/0x10b [btrfs]
[ 3915.143849]  [] __set_extent_bit+0x107/0x3f4 [btrfs]
[ 3915.145120]  [] ? btrfs_kill_super+0x17/0x23 [btrfs]
[ 3915.146352]  [] ? deactivate_locked_super+0x3b/0x50
[ 3915.147565]  [] lock_extent_bits+0x65/0x1bf [btrfs]
[ 3915.148785]  [] ? _raw_write_unlock+0x28/0x33
[ 3915.149931]  [] btrfs_evict_inode+0x196/0x482 [btrfs]
[ 3915.151154]  [] evict+0xa0/0x148
[ 3915.152094]  [] dispose_list+0x39/0x43
[ 3915.153081]  [] evict_inodes+0xdc/0xeb
[ 3915.154062]  [] generic_shutdown_super+0x49/0xef
[ 3915.155193]  [] kill_anon_super+0x13/0x1e
[ 3915.156274]  [] btrfs_kill_super+0x17/0x23 [btrfs]
(...)
[ 3915.167404] ---[ end trace 14843d3e2e622fc2 ]---

So just bail out of the clone ioctl if the length of the region to clone
is zero, without locking any extent range, in order to prevent this issue
(same behaviour as a pwrite with a 0 length for example).

This is trivial to reproduce. For example, the steps for the test I just
made for fstests:

  mkfs.btrfs -f SCRATCH_DEV
  mount SCRATCH_DEV $SCRATCH_MNT

  touch $SCRATCH_MNT/foo
  touch $SCRATCH_MNT/bar

  $CLONER_PROG -s 0 -d 4096 -l 0 $SCRATCH_MNT/foo $SCRATCH_MNT/bar
  umount $SCRATCH_MNT

A test case for fstests follows soon.

Signed-off-by: Filipe Manana 
Reviewed-by: Omar Sandoval 
Signed-off-by: Chris Mason 
Signed-off-by: Jiri Slaby

btrfs: don't accept bare namespace as a valid xattr

2015-04-30T11:49:27+00:00

commit 3c3b04d10ff1811a27f86684ccd2f5ba6983211d upstream.

Due to insufficient check in btrfs_is_valid_xattr, this unexpectedly
works:

 $ touch file
 $ setfattr -n user. -v 1 file
 $ getfattr -d file
user.="1"

ie. the missing attribute name after the namespace.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=94291
Reported-by: William Douglas 
Signed-off-by: David Sterba 
Signed-off-by: Chris Mason 
Signed-off-by: Jiri Slaby

Btrfs: fix log tree corruption when fs mounted with -o discard

2015-04-30T11:49:27+00:00

commit dcc82f4783ad91d4ab654f89f37ae9291cdc846a upstream.

While committing a transaction we free the log roots before we write the
new super block. Freeing the log roots implies marking the disk location
of every node/leaf (metadata extent) as pinned before the new super block
is written. This is to prevent the disk location of log metadata extents
from being reused before the new super block is written, otherwise we
would have a corrupted log tree if before the new super block is written
a crash/reboot happens and the location of any log tree metadata extent
ended up being reused and rewritten.

Even though we pinned the log tree's metadata extents, we were issuing a
discard against them if the fs was mounted with the -o discard option,
resulting in corruption of the log tree if a crash/reboot happened before
writing the new super block - the next time the fs was mounted, during
the log replay process we would find nodes/leafs of the log btree with
a content full of zeroes, causing the process to fail and require the
use of the tool btrfs-zero-log to wipeout the log tree (and all data
previously fsynced becoming lost forever).

Fix this by not doing a discard when pinning an extent. The discard will
be done later when it's safe (after the new super block is committed) at
extent-tree.c:btrfs_finish_extent_commit().

Fixes: e688b7252f78 (Btrfs: fix extent pinning bugs in the tree log)
Signed-off-by: Filipe Manana 
Signed-off-by: Chris Mason 
Signed-off-by: Jiri Slaby