1. 18 10月, 2018 5 次提交
  2. 06 10月, 2018 2 次提交
    • D
      xfs: fix data corruption w/ unaligned reflink ranges · b3998900
      Dave Chinner 提交于
      When reflinking sub-file ranges, a data corruption can occur when
      the source file range includes a partial EOF block. This shares the
      unknown data beyond EOF into the second file at a position inside
      EOF, exposing stale data in the second file.
      
      XFS only supports whole block sharing, but we still need to
      support whole file reflink correctly.  Hence if the reflink
      request includes the last block of the souce file, only proceed with
      the reflink operation if it lands at or past the destination file's
      current EOF. If it lands within the destination file EOF, reject the
      entire request with -EINVAL and make the caller go the hard way.
      
      This avoids the data corruption vector, but also avoids disruption
      of returning EINVAL to userspace for the common case of whole file
      cloning.
      Signed-off-by: NDave Chinner <dchinner@redhat.com>
      Reviewed-by: NDarrick J. Wong <darrick.wong@oracle.com>
      Signed-off-by: NDave Chinner <david@fromorbit.com>
      b3998900
    • D
      xfs: fix data corruption w/ unaligned dedupe ranges · dceeb47b
      Dave Chinner 提交于
      A deduplication data corruption is Exposed by fstests generic/505 on
      XFS. It is caused by extending the block match range to include the
      partial EOF block, but then allowing unknown data beyond EOF to be
      considered a "match" to data in the destination file because the
      comparison is only made to the end of the source file. This corrupts
      the destination file when the source extent is shared with it.
      
      XFS only supports whole block dedupe, but we still need to appear to
      support whole file dedupe correctly.  Hence if the dedupe request
      includes the last block of the souce file, don't include it in the
      actual XFS dedupe operation. If the rest of the range dedupes
      successfully, then report the partial last block as deduped, too, so
      that userspace sees it as a successful dedupe rather than return
      EINVAL because we can't dedupe unaligned blocks.
      Signed-off-by: NDave Chinner <dchinner@redhat.com>
      Reviewed-by: NDarrick J. Wong <darrick.wong@oracle.com>
      Signed-off-by: NDave Chinner <david@fromorbit.com>
      dceeb47b
  3. 05 10月, 2018 3 次提交
  4. 01 10月, 2018 1 次提交
    • D
      xfs: fix error handling in xfs_bmap_extents_to_btree · e55ec4dd
      Dave Chinner 提交于
      Commit 01239d77 ("xfs: fix a null pointer dereference in
      xfs_bmap_extents_to_btree") attempted to fix a null pointer
      dreference when a fuzzing corruption of some kind was found.
      This fix was flawed, resulting in assert failures like:
      
      XFS: Assertion failed: ifp->if_broot == NULL, file: fs/xfs/libxfs/xfs_bmap.c, line: 715
      .....
      Call Trace:
        xfs_bmap_extents_to_btree+0x6b9/0x7b0
        __xfs_bunmapi+0xae7/0xf00
        ? xfs_log_reserve+0x1c8/0x290
        xfs_reflink_remap_extent+0x20b/0x620
        xfs_reflink_remap_blocks+0x7e/0x290
        xfs_reflink_remap_range+0x311/0x530
        vfs_dedupe_file_range_one+0xd7/0xe0
        vfs_dedupe_file_range+0x15b/0x1a0
        do_vfs_ioctl+0x267/0x6c0
      
      The problem is that the error handling code now asserts that the
      inode fork is not in btree format before the error handling code
      undoes the modifications that put the fork back in extent format.
      Fix this by moving the assert back to after the xfs_iroot_realloc()
      call that returns the fork to extent format, and clean up the jump
      labels to be meaningful.
      
      Also, returning ENOSPC when xfs_btree_get_bufl() fails to
      instantiate the buffer that was allocated (the actual fix in the
      commit mentioned above) is incorrect. This is a fatal error - only
      an invalid block address or a filesystem shutdown can result in
      failing to get a buffer here.
      
      Hence change this to EFSCORRUPTED so that the higher layer knows
      this was a corruption related failure and should not treat it as an
      ENOSPC error.  This should result in a shutdown (via cancelling a
      dirty transaction) which is necessary as we do not attempt to clean
      up the (invalid) block that we have already allocated.
      Signed-off-by: NDave Chinner <dchinner@redhat.com>
      Reviewed-by: NDarrick J. Wong <darrick.wong@oracle.com>
      Signed-off-by: NDave Chinner <david@fromorbit.com>
      
      e55ec4dd
  5. 29 9月, 2018 14 次提交
    • B
      iomap: set page dirty after partial delalloc on mkwrite · 561295a3
      Brian Foster 提交于
      The iomap page fault mechanism currently dirties the associated page
      after the full block range of the page has been allocated. This
      leaves the page susceptible to delayed allocations without ever
      being set dirty on sub-page block sized filesystems.
      
      For example, consider a page fault on a page with one preexisting
      real (non-delalloc) block allocated in the middle of the page. The
      first iomap_apply() iteration performs delayed allocation on the
      range up to the preexisting block, the next iteration finds the
      preexisting block, and the last iteration attempts to perform
      delayed allocation on the range after the prexisting block to the
      end of the page. If the first allocation succeeds and the final
      allocation fails with -ENOSPC, iomap_apply() returns the error and
      iomap_page_mkwrite() fails to dirty the page having already
      performed partial delayed allocation. This eventually results in the
      page being invalidated without ever converting the delayed
      allocation to real blocks.
      
      This problem is reliably reproduced by generic/083 on XFS on ppc64
      systems (64k page size, 4k block size). It results in leaked
      delalloc blocks on inode reclaim, which triggers an assert failure
      in xfs_fs_destroy_inode() and filesystem accounting inconsistency.
      
      Move the set_page_dirty() call from iomap_page_mkwrite() to the
      actor callback, similar to how the buffer head implementation works.
      The actor callback is called iff ->iomap_begin() returns success, so
      ensures the page is dirtied as soon as possible after an allocation.
      Signed-off-by: NBrian Foster <bfoster@redhat.com>
      Reviewed-by: NDave Chinner <dchinner@redhat.com>
      Signed-off-by: NDave Chinner <david@fromorbit.com>
      561295a3
    • B
      xfs: remove invalid log recovery first/last cycle check · ec2ed0b5
      Brian Foster 提交于
      One of the first steps of log recovery is to check for the special
      case of a zeroed log. If the first cycle in the log is zero or the
      tail portion of the log is zeroed, the head is set to the first
      instance of cycle 0. xlog_find_zeroed() includes a sanity check that
      enforces that the first cycle in the log must be 1 if the last cycle
      is 0. While this is true in most cases, the check is not totally
      valid because it doesn't consider the case where the filesystem
      crashed after a partial/out of order log buffer completion that
      wraps around the end of the physical log.
      
      For example, consider a filesystem that has completed most of the
      first cycle of the log, reaches the end of the physical log and
      splits the next single log buffer write into two in order to wrap
      around the end of the log. If these I/Os are reordered, the second
      (wrapped) I/O completes and the first happens to fail, the log is
      left in a state where the last cycle of the log is 0 and the first
      cycle is 2. This causes the xlog_find_zeroed() sanity check to fail
      and prevents the filesystem from mounting. This situation has been
      reproduced on particular systems via repeated runs of generic/475.
      
      This is an expected state that log recovery already knows how to
      deal with, however. Since the log is still partially zeroed, the
      head is detected correctly and points to a valid tail. The
      subsequent stale block detection clears blocks beyond the head up to
      the tail (within a maximum range), with the express purpose of
      clearing such out of order writes. As expected, this removes the out
      of order cycle 2 blocks at the physical start of the log.
      
      In other words, the only thing that prevents a clean mount and
      recovery of the filesystem in this scenario is the specific (last ==
      0 && first != 1) sanity check in xlog_find_zeroed(). Since the log
      head/tail are now independently validated via cycle, log record and
      CRC checks, this highly specific first cycle check is of dubious
      value. Remove it and rely on the higher level validation to
      determine whether log content is sane and recoverable.
      Signed-off-by: NBrian Foster <bfoster@redhat.com>
      Reviewed-by: NDave Chinner <dchinner@redhat.com>
      Signed-off-by: NDave Chinner <david@fromorbit.com>
      
      ec2ed0b5
    • E
      xfs: validate inode di_forkoff · 339e1a3f
      Eric Sandeen 提交于
      Verify the inode di_forkoff, lifted from xfs_repair's
      process_check_inode_forkoff().
      Signed-off-by: NEric Sandeen <sandeen@redhat.com>
      Reviewed-by: NBrian Foster <bfoster@redhat.com>
      Signed-off-by: NDave Chinner <david@fromorbit.com>
      339e1a3f
    • C
      xfs: skip delalloc COW blocks in xfs_reflink_end_cow · f5f3f959
      Christoph Hellwig 提交于
      The iomap direct I/O code issues a single ->end_io call for the whole
      I/O request, and if some of the extents cowered needed a COW operation
      it will call xfs_reflink_end_cow over the whole range.
      
      When we do AIO writes we drop the iolock after doing the initial setup,
      but before the I/O completion.  Between dropping the lock and completing
      the I/O we can have a racing buffered write create new delalloc COW fork
      extents in the region covered by the outstanding direct I/O write, and
      thus see delalloc COW fork extents in xfs_reflink_end_cow.  As
      concurrent writes are fundamentally racy and no guarantees are given we
      can simply skip those.
      
      This can be easily reproduced with xfstests generic/208 in always_cow
      mode.
      Signed-off-by: NChristoph Hellwig <hch@lst.de>
      Reviewed-by: NDarrick J. Wong <darrick.wong@oracle.com>
      Signed-off-by: NDave Chinner <david@fromorbit.com>
      
      f5f3f959
    • E
      xfs: don't treat unknown di_flags2 as corruption in scrub · f369a13c
      Eric Sandeen 提交于
      xchk_inode_flags2() currently treats any di_flags2 values that the
      running kernel doesn't recognize as corruption, and calls
      xchk_ino_set_corrupt() if they are set.  However, it's entirely possible
      that these flags were set in some newer kernel and are quite valid,
      but ignored in this kernel.
      
      (Validators don't care one bit about unknown di_flags2.)
      
      Call xchk_ino_set_warning instead, because this may or may not actually
      indicate a problem.
      Signed-off-by: NEric Sandeen <sandeen@redhat.com>
      Reviewed-by: NDarrick J. Wong <darrick.wong@oracle.com>
      Signed-off-by: NDave Chinner <david@fromorbit.com>
      f369a13c
    • Y
      xfs: remove duplicated include from alloc.c · 2863c2eb
      YueHaibing 提交于
      Remove duplicated include xfs_alloc.h
      Signed-off-by: NYueHaibing <yuehaibing@huawei.com>
      Reviewed-by: NDarrick J. Wong <darrick.wong@oracle.com>
      Signed-off-by: NDave Chinner <david@fromorbit.com>
      
      2863c2eb
    • C
      xfs: don't bring in extents in xfs_bmap_punch_delalloc_range · 0065b541
      Christoph Hellwig 提交于
      This function is only used to punch out delayed allocations on I/O
      failure, which means we need to have read the extents earlier.
      Signed-off-by: NChristoph Hellwig <hch@lst.de>
      Reviewed-by: NDarrick J. Wong <darrick.wong@oracle.com>
      Signed-off-by: NDave Chinner <david@fromorbit.com>
      0065b541
    • D
      xfs: fix transaction leak in xfs_reflink_allocate_cow() · df307077
      Dave Chinner 提交于
      When xfs_reflink_allocate_cow() allocates a transaction, it drops
      the ILOCK to perform the operation. This Introduces a race condition
      where another thread modifying the file can perform the COW
      allocation operation underneath us. This result in the retry loop
      finding an allocated block and jumping straight to the conversion
      code. It does not, however, cancel the transaction it holds and so
      this gets leaked. This results in a lockdep warning:
      
      ================================================
      WARNING: lock held when returning to user space!
      4.18.5 #1 Not tainted
      ------------------------------------------------
      worker/6123 is leaving the kernel with locks still held!
      1 lock held by worker/6123:
       #0: 000000009eab4f1b (sb_internal#2){.+.+}, at: xfs_trans_alloc+0x17c/0x220
      
      And eventually the filesystem deadlocks because it runs out of log
      space that is reserved by the leaked transaction and never gets
      released.
      
      The logic flow in xfs_reflink_allocate_cow() is a convoluted mess of
      gotos - it's no surprise that it has bug where the flow through
      several goto jumps then fails to clean up context from a non-obvious
      logic path. CLean up the logic flow and make sure every path does
      the right thing.
      Reported-by: NAlexander Y. Fomichev <git.user@gmail.com>
      Tested-by: NAlexander Y. Fomichev <git.user@gmail.com>
      Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=200981Signed-off-by: NDave Chinner <dchinner@redhat.com>
      [hch: slight refactor]
      Signed-off-by: NChristoph Hellwig <hch@lst.de>
      Reviewed-by: NDarrick J. Wong <darrick.wong@oracle.com>
      Signed-off-by: NDave Chinner <david@fromorbit.com>
      df307077
    • D
      xfs: avoid lockdep false positives in xfs_trans_alloc · 8683edb7
      Dave Chinner 提交于
      We've had a few reports of lockdep tripping over memory reclaim
      context vs filesystem freeze "deadlocks". They all have looked
      to be false positives on analysis, but it seems that they are
      being tripped because we take freeze references before we run
      a GFP_KERNEL allocation for the struct xfs_trans.
      
      We can avoid this false positive vector just by re-ordering the
      operations in xfs_trans_alloc(). That is. we need allocate the
      structure before we take the freeze reference and enter the GFP_NOFS
      allocation context that follows the xfs_trans around. This prevents
      lockdep from seeing the GFP_KERNEL allocation inside the transaction
      context, and that prevents it from triggering the freeze level vs
      alloc context vs reclaim warnings.
      Signed-off-by: NDave Chinner <dchinner@redhat.com>
      Reviewed-by: NBrian Foster <bfoster@redhat.com>
      Reviewed-by: NChristoph Hellwig <hch@lst.de>
      8683edb7
    • B
      xfs: refactor xfs_buf_log_item reference count handling · 95808459
      Brian Foster 提交于
      The xfs_buf_log_item structure has a reference counter with slightly
      tricky semantics. In the common case, a buffer is logged and
      committed in a transaction, committed to the on-disk log (added to
      the AIL) and then finally written back and removed from the AIL. The
      bli refcount covers two potentially overlapping timeframes:
      
       1. the bli is held in an active transaction
       2. the bli is pinned by the log
      
      The caveat to this approach is that the reference counter does not
      purely dictate the lifetime of the bli. IOW, when a dirty buffer is
      physically logged and unpinned, the bli refcount may go to zero as
      the log item is inserted into the AIL. Only once the buffer is
      written back can the bli finally be freed.
      
      The above semantics means that it is not enough for the various
      refcount decrementing contexts to release the bli on decrement to
      zero. xfs_trans_brelse(), transaction commit (->iop_unlock()) and
      unpin (->iop_unpin()) must all drop the associated reference and
      make additional checks to determine if the current context is
      responsible for freeing the item.
      
      For example, if a transaction holds but does not dirty a particular
      bli, the commit may drop the refcount to zero. If the bli itself is
      clean, it is also not AIL resident and must be freed at this time.
      The same is true for xfs_trans_brelse(). If the transaction dirties
      a bli and then aborts or an unpin results in an abort due to a log
      I/O error, the last reference count holder is expected to explicitly
      remove the item from the AIL and release it (since an abort means
      filesystem shutdown and metadata writeback will never occur).
      
      This leads to fairly complex checks being replicated in a few
      different places. Since ->iop_unlock() and xfs_trans_brelse() are
      nearly identical, refactor the logic into a common helper that
      implements and documents the semantics in one place. This patch does
      not change behavior.
      Signed-off-by: NBrian Foster <bfoster@redhat.com>
      Reviewed-by: NDave Chinner <dchinner@redhat.com>
      Signed-off-by: NDave Chinner <david@fromorbit.com>
      
      95808459
    • B
      xfs: clean up xfs_trans_brelse() · 23420d05
      Brian Foster 提交于
      xfs_trans_brelse() is a bit of a historical mess, similar to
      xfs_buf_item_unlock(). It is unnecessarily verbose, has snippets of
      commented out code, inconsistency with regard to stale items, etc.
      
      Clean up xfs_trans_brelse() to use similar logic and flow as
      xfs_buf_item_unlock() with regard to bli reference count handling.
      This patch makes no functional changes, but facilitates further
      refactoring of the common bli reference count handling code.
      Signed-off-by: NBrian Foster <bfoster@redhat.com>
      Reviewed-by: NDave Chinner <dchinner@redhat.com>
      Signed-off-by: NDave Chinner <david@fromorbit.com>
      
      23420d05
    • B
      xfs: don't unlock invalidated buf on aborted tx commit · d9183105
      Brian Foster 提交于
      xfstests generic/388,475 occasionally reproduce assertion failures
      in xfs_buf_item_unpin() when the final bli reference is dropped on
      an invalidated buffer and the buffer is not locked as it is expected
      to be. Invalidated buffers should remain locked on transaction
      commit until the final unpin, at which point the buffer is removed
      from the AIL and the bli is freed since stale buffers are not
      written back.
      
      The assert failures are associated with filesystem shutdown,
      typically due to log I/O errors injected by the test. The
      problematic situation can occur if the shutdown happens to cause a
      race between an active transaction that has invalidated a particular
      buffer and an I/O error on a log buffer that contains the bli
      associated with the same (now stale) buffer.
      
      Both transaction and log contexts acquire a bli reference. If the
      transaction has already invalidated the buffer by the time the I/O
      error occurs and ends up aborting due to shutdown, the transaction
      and log hold the last two references to a stale bli. If the
      transaction cancel occurs first, it treats the buffer as non-stale
      due to the aborted state: the bli reference is dropped and the
      buffer is released/unlocked. The log buffer I/O error handling
      eventually calls into xfs_buf_item_unpin(), drops the final
      reference to the bli and treats it as stale. The buffer wasn't left
      locked by xfs_buf_item_unlock(), however, so the assert fails and
      the buffer is double unlocked. The latter problem is mitigated by
      the fact that the fs is shutdown and no further damage is possible.
      
      ->iop_unlock() of an invalidated buffer should behave consistently
      with respect to the bli refcount, regardless of aborted state. If
      the refcount remains elevated on commit, we know the bli is awaiting
      an unpin (since it can't be in another transaction) and will be
      handled appropriately on log buffer completion. If the final bli
      reference of an invalidated buffer is dropped in ->iop_unlock(), we
      can assume the transaction has aborted because invalidation implies
      a dirty transaction. In the non-abort case, the log would have
      acquired a bli reference in ->iop_pin() and prevented bli release at
      ->iop_unlock() time. In the abort case the item must be freed and
      buffer unlocked because it wasn't pinned by the log.
      
      Rework xfs_buf_item_unlock() to simplify the currently circuitous
      and duplicate logic and leave invalidated buffers locked based on
      bli refcount, regardless of aborted state. This ensures that a
      pinned, stale buffer is always found locked when eventually
      unpinned.
      Signed-off-by: NBrian Foster <bfoster@redhat.com>
      Reviewed-by: NDave Chinner <dchinner@redhat.com>
      Signed-off-by: NDave Chinner <david@fromorbit.com>
      d9183105
    • B
      xfs: remove last of unnecessary xfs_defer_cancel() callers · d5a2e289
      Brian Foster 提交于
      Now that deferred operations are completely managed via
      transactions, it's no longer necessary to cancel the dfops in error
      paths that already cancel the associated transaction. There are a
      few such calls lingering throughout the codebase.
      
      Remove all remaining unnecessary calls to xfs_defer_cancel(). This
      leaves xfs_defer_cancel() calls in two places. The first is the call
      in the transaction cancel path itself, which facilitates this patch.
      The second is made via the xfs_defer_finish() error path to provide
      consistent error semantics with transaction commit. For example,
      xfs_trans_commit() expects an xfs_defer_finish() failure to clean up
      the dfops structure before it returns.
      Signed-off-by: NBrian Foster <bfoster@redhat.com>
      Reviewed-by: NDave Chinner <dchinner@redhat.com>
      Signed-off-by: NDave Chinner <david@fromorbit.com>
      
      d5a2e289
    • D
      xfs: don't crash the vfs on a garbage inline symlink · ae294787
      Darrick J. Wong 提交于
      The VFS routine that calls ->get_link blindly copies whatever's returned
      into the user's buffer.  If we return a NULL pointer, the vfs will
      crash on the null pointer.  Therefore, return -EFSCORRUPTED instead of
      blowing up the kernel.
      
      [dgc: clean up with hch's suggestions]
      
      Reported-by: wen.xu@gatech.edu
      Signed-off-by: NDarrick J. Wong <darrick.wong@oracle.com>
      Reviewed-by: NAllison Henderson <allison.henderson@oracle.com>
      Signed-off-by: NDave Chinner <david@fromorbit.com>
      ae294787
  6. 26 8月, 2018 1 次提交
  7. 24 8月, 2018 13 次提交
  8. 23 8月, 2018 1 次提交