[PATCH] Btrfs: fix race when cleaning unused block groups

We have a race while deleting unused block groups that causes extents written
by past generations/transactions to be rewritten by the current transaction
before that transaction is committed. The steps that lead to this issue:

1) At transaction N one or more block groups became unused and we added them
   to the list fs_info->unused_bgs;

2) While still at transaction N we write btree extents to block group X and the
   transaction is committed;

3) The cleaner kthread is awaken and calls btrfs_delete_unused_bgs() to go through
   the list fs_info->unused_bgs and remove unused block groups;

4) Transaction N + 1 starts;

5) At transaction N + 1, block group X becomes unused and is added to the list
   fs_info->unused_bgs - this implies delayed refs were run, so we had the
   following function calls: btrfs_run_delayed_refs() -> __btrfs_free_extent()
   -> update_block_group(). The update_block_group() function grabs the lock
   fs_info->unused_bgs_lock, adds block group X to fs_info->unused_bgs and
   releases that lock;

6) The cleaner kthread, while at btrfs_delete_unused_bgs(), sees block group X
   added by transaction N + 1 because it's doing a loop that finishes only when
   the list fs_info->unused_bgs is empty and locks and unlocks the spinlock
   fs_info->unused_bgs_lock on each iteration. So it deletes the block group
   and its respective chunk is released. Even if it didn't do the lock/unlock
   per iteration, it could still see block group X in the list, because the
   cleaner kthread might call btrfs_delete_unused_bgs() multiple times (for
   example if there are several snapshots to delete);

7) A new block group X' is created for data, and it's associated to the same chunk
   that block group X was associated to;

8) Extents from block group X' are allocated for file data and for example an fsync
   makes the file data be effectively written to disk;

9) A crash/reboot happens before transaction N + 1 is committed;

10) On the next mount, we will read extents from block group/chunk X but they no
   longer have valid btree nodes/leafs - they have instead file data, and therefore
   all sorts of errors will happen.

So fix this by ensuring the cleaner kthread can never delete a block group that
became unused in the current transaction, that is, only delete block groups that
were added to the unused_bgs list by past transactions.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/ctree.h       | 1 +
 fs/btrfs/disk-io.c     | 1 +
 fs/btrfs/extent-tree.c | 5 +++--
 fs/btrfs/transaction.c | 5 +++++
 4 files changed, 10 insertions(+), 2 deletions(-)
[PATCH 1/4] btrfs: more superblock checks, lower bounds on devices and sectorsize_nodesize

I received a few crafted images from Jiri, all got through the recently
added superblock checks. The lower bounds checks for num_devices and
sector/node -sizes were missing and caused a crash during mount.

Tools for symbolic code execution were used to prepare the images
contents.

Reported-by: Jiri Slaby <jslaby@suse.cz>
Signed-off-by: David Sterba <dsterba@suse.cz>
---
 fs/btrfs/disk-io.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
[PATCH 2/4] btrfs: add checks for sys_chunk_array sizes

Verify that possible minimum and maximum size is set, validity of
contents is checked in btrfs_read_sys_array.

Signed-off-by: David Sterba <dsterba@suse.cz>
---
 fs/btrfs/disk-io.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
[PATCH 3/4] btrfs: cleanup, rename a few variables in btrfs_read_sys_array

There's a pointer to buffer, integer offset and offset passed as
pointer, try to find matching names for them.

Signed-off-by: David Sterba <dsterba@suse.cz>
---
 fs/btrfs/volumes.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)
[PATCH 4/4] btrfs: add more checks to btrfs_read_sys_array

Verify that the sys_array has enough bytes to read the next item.

Signed-off-by: David Sterba <dsterba@suse.cz>
---
 fs/btrfs/volumes.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)
[PATCH] Btrfs: make xattr replace operations atomic

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 <oliva@gnu.org>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/ctree.h    |   4 ++
 fs/btrfs/dir-item.c |  10 ++--
 fs/btrfs/xattr.c    | 142 ++++++++++++++++++++++++++++++----------------------
 3 files changed, 88 insertions(+), 68 deletions(-)