1. 22 Feb, 2021 10 commits
    • Josef Bacik's avatar
      btrfs: avoid double put of block group when emptying cluster · 95c85fba
      Josef Bacik authored
      It's wrong calling btrfs_put_block_group in
      __btrfs_return_cluster_to_free_space if the block group passed is
      different than the block group the cluster represents. As this means the
      cluster doesn't have a reference to the passed block group. This results
      in double put and a use-after-free bug.
      
      Fix this by simply bailing if the block group we passed in does not
      match the block group on the cluster.
      
      Fixes: fa9c0d79 ("Btrfs: rework allocation clustering")
      CC: stable@vger.kernel.org # 4.4+
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      [ update changelog ]
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      95c85fba
    • Filipe Manana's avatar
      btrfs: fix stale data exposure after cloning a hole with NO_HOLES enabled · 3660d0bc
      Filipe Manana authored
      When using the NO_HOLES feature, if we clone a file range that spans only
      a hole into a range that is at or beyond the current i_size of the
      destination file, we end up not setting the full sync runtime flag on the
      inode. As a result, if we then fsync the destination file and have a power
      failure, after log replay we can end up exposing stale data instead of
      having a hole for that range.
      
      The conditions for this to happen are the following:
      
      1) We have a file with a size of, for example, 1280K;
      
      2) There is a written (non-prealloc) extent for the file range from 1024K
         to 1280K with a length of 256K;
      
      3) This particular file extent layout is durably persisted, so that the
         existing superblock persisted on disk points to a subvolume root where
         the file has that exact file extent layout and state;
      
      4) The file is truncated to a smaller size, to an offset lower than the
         start offset of its last extent, for example to 800K. The truncate sets
         the full sync runtime flag on the inode;
      
      6) Fsync the file to log it and clear the full sync runtime flag;
      
      7) Clone a region that covers only a hole (implicit hole due to NO_HOLES)
         into the file with a destination offset that starts at or beyond the
         256K file extent item we had - for example to offset 1024K;
      
      8) Since the clone operation does not find extents in the source range,
         we end up in the if branch at the bottom of btrfs_clone() where we
         punch a hole for the file range starting at offset 1024K by calling
         btrfs_replace_file_extents(). There we end up not setting the full
         sync flag on the inode, because we don't know we are being called in
         a clone context (and not fallocate's punch hole operation), and
         neither do we create an extent map to represent a hole because the
         requested range is beyond eof;
      
      9) A further fsync to the file will be a fast fsync, since the clone
         operation did not set the full sync flag, and therefore it relies on
         modified extent maps to correctly log the file layout. But since
         it does not find any extent map marking the range from 1024K (the
         previous eof) to the new eof, it does not log a file extent item
         for that range representing the hole;
      
      10) After a power failure no hole for the range starting at 1024K is
         punched and we end up exposing stale data from the old 256K extent.
      
      Turning this into exact steps:
      
        $ mkfs.btrfs -f -O no-holes /dev/sdi
        $ mount /dev/sdi /mnt
      
        # Create our test file with 3 extents of 256K and a 256K hole at offset
        # 256K. The file has a size of 1280K.
        $ xfs_io -f -s \
                    -c "pwrite -S 0xab -b 256K 0 256K" \
                    -c "pwrite -S 0xcd -b 256K 512K 256K" \
                    -c "pwrite -S 0xef -b 256K 768K 256K" \
                    -c "pwrite -S 0x73 -b 256K 1024K 256K" \
                    /mnt/sdi/foobar
      
        # Make sure it's durably persisted. We want the last committed super
        # block to point to this particular file extent layout.
        sync
      
        # Now truncate our file to a smaller size, falling within a position of
        # the second extent. This sets the full sync runtime flag on the inode.
        # Then fsync the file to log it and clear the full sync flag from the
        # inode. The third extent is no longer part of the file and therefore
        # it is not logged.
        $ xfs_io -c "truncate 800K" -c "fsync" /mnt/foobar
      
        # Now do a clone operation that only clones the hole and sets back the
        # file size to match the size it had before the truncate operation
        # (1280K).
        $ xfs_io \
              -c "reflink /mnt/foobar 256K 1024K 256K" \
              -c "fsync" \
              /mnt/foobar
      
        # File data before power failure:
        $ od -A d -t x1 /mnt/foobar
        0000000 ab ab ab ab ab ab ab ab ab ab ab ab ab ab ab ab
        *
        0262144 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
        *
        0524288 cd cd cd cd cd cd cd cd cd cd cd cd cd cd cd cd
        *
        0786432 ef ef ef ef ef ef ef ef ef ef ef ef ef ef ef ef
        *
        0819200 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
        *
        1310720
      
        <power fail>
      
        # Mount the fs again to replay the log tree.
        $ mount /dev/sdi /mnt
      
        # File data after power failure:
        $ od -A d -t x1 /mnt/foobar
        0000000 ab ab ab ab ab ab ab ab ab ab ab ab ab ab ab ab
        *
        0262144 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
        *
        0524288 cd cd cd cd cd cd cd cd cd cd cd cd cd cd cd cd
        *
        0786432 ef ef ef ef ef ef ef ef ef ef ef ef ef ef ef ef
        *
        0819200 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
        *
        1048576 73 73 73 73 73 73 73 73 73 73 73 73 73 73 73 73
        *
        1310720
      
      The range from 1024K to 1280K should correspond to a hole but instead it
      points to stale data, to the 256K extent that should not exist after the
      truncate operation.
      
      The issue does not exists when not using NO_HOLES, because for that case
      we use file extent items to represent holes, these are found and copied
      during the loop that iterates over extents at btrfs_clone(), and that
      causes btrfs_replace_file_extents() to be called with a non-NULL
      extent_info argument and therefore set the full sync runtime flag on the
      inode.
      
      So fix this by making the code that deals with a trailing hole during
      cloning, at btrfs_clone(), to set the full sync flag on the inode, if the
      range starts at or beyond the current i_size.
      
      A test case for fstests will follow soon.
      
      Backporting notes: for kernel 5.4 the change goes to ioctl.c into
      btrfs_clone before the last call to btrfs_punch_hole_range.
      
      CC: stable@vger.kernel.org # 5.4+
      Reviewed-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      3660d0bc
    • Josef Bacik's avatar
      btrfs: tree-checker: do not error out if extent ref hash doesn't match · 1119a72e
      Josef Bacik authored
      The tree checker checks the extent ref hash at read and write time to
      make sure we do not corrupt the file system.  Generally extent
      references go inline, but if we have enough of them we need to make an
      item, which looks like
      
      key.objectid	= <bytenr>
      key.type	= <BTRFS_EXTENT_DATA_REF_KEY|BTRFS_TREE_BLOCK_REF_KEY>
      key.offset	= hash(tree, owner, offset)
      
      However if key.offset collide with an unrelated extent reference we'll
      simply key.offset++ until we get something that doesn't collide.
      Obviously this doesn't match at tree checker time, and thus we error
      while writing out the transaction.  This is relatively easy to
      reproduce, simply do something like the following
      
        xfs_io -f -c "pwrite 0 1M" file
        offset=2
      
        for i in {0..10000}
        do
      	  xfs_io -c "reflink file 0 ${offset}M 1M" file
      	  offset=$(( offset + 2 ))
        done
      
        xfs_io -c "reflink file 0 17999258914816 1M" file
        xfs_io -c "reflink file 0 35998517829632 1M" file
        xfs_io -c "reflink file 0 53752752058368 1M" file
      
        btrfs filesystem sync
      
      And the sync will error out because we'll abort the transaction.  The
      magic values above are used because they generate hash collisions with
      the first file in the main subvol.
      
      The fix for this is to remove the hash value check from tree checker, as
      we have no idea which offset ours should belong to.
      Reported-by: default avatarTuomas Lähdekorpi <tuomas.lahdekorpi@gmail.com>
      Fixes: 0785a9aa ("btrfs: tree-checker: Add EXTENT_DATA_REF check")
      CC: stable@vger.kernel.org # 5.4+
      Reviewed-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      [ add comment]
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      1119a72e
    • Filipe Manana's avatar
      btrfs: fix race between swap file activation and snapshot creation · dd0734f2
      Filipe Manana authored
      When creating a snapshot we check if the current number of swap files, in
      the root, is non-zero, and if it is, we error out and warn that we can not
      create the snapshot because there are active swap files.
      
      However this is racy because when a task started activation of a swap
      file, another task might have started already snapshot creation and might
      have seen the counter for the number of swap files as zero. This means
      that after the swap file is activated we may end up with a snapshot of the
      same root successfully created, and therefore when the first write to the
      swap file happens it has to fall back into COW mode, which should never
      happen for active swap files.
      
      Basically what can happen is:
      
      1) Task A starts snapshot creation and enters ioctl.c:create_snapshot().
         There it sees that root->nr_swapfiles has a value of 0 so it continues;
      
      2) Task B enters btrfs_swap_activate(). It is not aware that another task
         started snapshot creation but it did not finish yet. It increments
         root->nr_swapfiles from 0 to 1;
      
      3) Task B checks that the file meets all requirements to be an active
         swap file - it has NOCOW set, there are no snapshots for the inode's
         root at the moment, no file holes, no reflinked extents, etc;
      
      4) Task B returns success and now the file is an active swap file;
      
      5) Task A commits the transaction to create the snapshot and finishes.
         The swap file's extents are now shared between the original root and
         the snapshot;
      
      6) A write into an extent of the swap file is attempted - there is a
         snapshot of the file's root, so we fall back to COW mode and therefore
         the physical location of the extent changes on disk.
      
      So fix this by taking the snapshot lock during swap file activation before
      locking the extent range, as that is the order in which we lock these
      during buffered writes.
      
      Fixes: ed46ff3d ("Btrfs: support swap files")
      CC: stable@vger.kernel.org # 5.4+
      Reviewed-by: default avatarAnand Jain <anand.jain@oracle.com>
      Reviewed-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      dd0734f2
    • Filipe Manana's avatar
      btrfs: fix race between writes to swap files and scrub · 195a49ea
      Filipe Manana authored
      When we active a swap file, at btrfs_swap_activate(), we acquire the
      exclusive operation lock to prevent the physical location of the swap
      file extents to be changed by operations such as balance and device
      replace/resize/remove. We also call there can_nocow_extent() which,
      among other things, checks if the block group of a swap file extent is
      currently RO, and if it is we can not use the extent, since a write
      into it would result in COWing the extent.
      
      However we have no protection against a scrub operation running after we
      activate the swap file, which can result in the swap file extents to be
      COWed while the scrub is running and operating on the respective block
      group, because scrub turns a block group into RO before it processes it
      and then back again to RW mode after processing it. That means an attempt
      to write into a swap file extent while scrub is processing the respective
      block group, will result in COWing the extent, changing its physical
      location on disk.
      
      Fix this by making sure that block groups that have extents that are used
      by active swap files can not be turned into RO mode, therefore making it
      not possible for a scrub to turn them into RO mode. When a scrub finds a
      block group that can not be turned to RO due to the existence of extents
      used by swap files, it proceeds to the next block group and logs a warning
      message that mentions the block group was skipped due to active swap
      files - this is the same approach we currently use for balance.
      
      Fixes: ed46ff3d ("Btrfs: support swap files")
      CC: stable@vger.kernel.org # 5.4+
      Reviewed-by: default avatarAnand Jain <anand.jain@oracle.com>
      Reviewed-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      195a49ea
    • Filipe Manana's avatar
      btrfs: avoid checking for RO block group twice during nocow writeback · 20903032
      Filipe Manana authored
      During the nocow writeback path, we currently iterate the rbtree of block
      groups twice: once for checking if the target block group is RO with the
      call to btrfs_extent_readonly()), and once again for getting a nocow
      reference on the block group with a call to btrfs_inc_nocow_writers().
      
      Since btrfs_inc_nocow_writers() already returns false when the target
      block group is RO, remove the call to btrfs_extent_readonly(). Not only
      we avoid searching the blocks group rbtree twice, it also helps reduce
      contention on the lock that protects it (specially since it is a spin
      lock and not a read-write lock). That may make a noticeable difference
      on very large filesystems, with thousands of allocated block groups.
      Reviewed-by: default avatarAnand Jain <anand.jain@oracle.com>
      Reviewed-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      20903032
    • Nikolay Borisov's avatar
      btrfs: fix race between extent freeing/allocation when using bitmaps · 3c179165
      Nikolay Borisov authored
      During allocation the allocator will try to allocate an extent using
      cluster policy. Once the current cluster is exhausted it will remove the
      entry under btrfs_free_cluster::lock and subsequently acquire
      btrfs_free_space_ctl::tree_lock to dispose of the already-deleted entry
      and adjust btrfs_free_space_ctl::total_bitmap. This poses a problem
      because there exists a race condition between removing the entry under
      one lock and doing the necessary accounting holding a different lock
      since extent freeing only uses the 2nd lock. This can result in the
      following situation:
      
      T1:                                    T2:
      btrfs_alloc_from_cluster               insert_into_bitmap <holds tree_lock>
       if (entry->bytes == 0)                   if (block_group && !list_empty(&block_group->cluster_list)) {
          rb_erase(entry)
      
       spin_unlock(&cluster->lock);
         (total_bitmaps is still 4)           spin_lock(&cluster->lock);
                                               <doesn't find entry in cluster->root>
       spin_lock(&ctl->tree_lock);             <goes to new_bitmap label, adds
      <blocked since T2 holds tree_lock>       <a new entry and calls add_new_bitmap>
      					    recalculate_thresholds  <crashes,
                                                    due to total_bitmaps
      					      becoming 5 and triggering
      					      an ASSERT>
      
      To fix this ensure that once depleted, the cluster entry is deleted when
      both cluster lock and tree locks are held in the allocator (T1), this
      ensures that even if there is a race with a concurrent
      insert_into_bitmap call it will correctly find the entry in the cluster
      and add the new space to it.
      
      CC: <stable@vger.kernel.org> # 4.4+
      Reviewed-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarNikolay Borisov <nborisov@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      3c179165
    • Qu Wenruo's avatar
      btrfs: make check_compressed_csum() to be subpage compatible · 04d4ba4c
      Qu Wenruo authored
      Currently check_compressed_csum() completely relies on sectorsize ==
      PAGE_SIZE to do checksum verification for compressed extents.
      
      To make it subpage compatible, this patch will:
      - Do extra calculation for the csum range
        Since we have multiple sectors inside a page, we need to only hash
        the range we want, not the full page anymore.
      
      - Do sector-by-sector hash inside the page
      
      With this patch and previous conversion on
      btrfs_submit_compressed_read(), now we can read subpage compressed
      extents properly, and do proper csum verification.
      Reviewed-by: default avatarAnand Jain <anand.jain@oracle.com>
      Signed-off-by: default avatarQu Wenruo <wqu@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      04d4ba4c
    • Qu Wenruo's avatar
      btrfs: make btrfs_submit_compressed_read() subpage compatible · be6a1361
      Qu Wenruo authored
      For compressed read, we always submit page read using page size.  This
      doesn't work well with subpage, as for subpage one page can contain
      several sectors.  Such submission will read range out of what we want,
      and cause problems.
      
      Thankfully to make it subpage compatible, we only need to change how the
      last page of the compressed extent is read.
      
      Instead of always adding a full page to the compressed read bio, if we're
      at the last page, calculate the size using compressed length, so that we
      only add part of the range into the compressed read bio.
      
      Since we are here, also change the PAGE_SIZE used in
      lookup_extent_mapping() to sectorsize.
      This modification won't cause any functional change, as
      lookup_extent_mapping() can handle the case where the search range is
      larger than found extent range.
      Reviewed-by: default avatarAnand Jain <anand.jain@oracle.com>
      Signed-off-by: default avatarQu Wenruo <wqu@suse.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      be6a1361
    • Ira Weiny's avatar
      btrfs: fix raid6 qstripe kmap · d70cef0d
      Ira Weiny authored
      When a qstripe is required an extra page is allocated and mapped.  There
      were 3 problems:
      
      1) There is no corresponding call of kunmap() for the qstripe page.
      2) There is no reason to map the qstripe page more than once if the
         number of bits set in rbio->dbitmap is greater than one.
      3) There is no reason to map the parity page and unmap it each time
         through the loop.
      
      The page memory can continue to be reused with a single mapping on each
      iteration by raid6_call.gen_syndrome() without remapping.  So map the
      page for the duration of the loop.
      
      Similarly, improve the algorithm by mapping the parity page just 1 time.
      
      Fixes: 5a6ac9ea ("Btrfs, raid56: support parity scrub on raid56")
      CC: stable@vger.kernel.org # 4.4.x: c17af965: btrfs: raid56: simplify tracking of Q stripe presence
      CC: stable@vger.kernel.org # 4.4.x
      Signed-off-by: default avatarIra Weiny <ira.weiny@intel.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      d70cef0d
  2. 09 Feb, 2021 30 commits