LOLLM noticed that the inode btree root formatting methods copy too many
bytes -- there's only one set of keys in node blocks, not two. This
causes memory corruption of whatever's beyond the buffers.
Cc: stable@vger.kernel.org # v6.14
Fixes: f0415af60f ("xfs: wire up a new metafile type for the realtime refcount")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM points out that if the amount of fdblocks that we can reserve for
a metadata btree file goes below the space already used by that file,
then the hidden_space subtraction can underflow, causing
xfs_dec_fdblocks to subtract a huge amount of space. We never want the
target to be less than the used sapce, so fix the logic that adjusts
dblocks_avail downwards.
Also fix an error in the adjacent comment.
Cc: stable@vger.kernel.org # v6.15
Fixes: 1df8d75030 ("xfs: make metabtree reservations global")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM noticed that xfs_qm_flush_one fails to drop the dquot flush lock
if it can't grab the buffer associated with the dquot. Since there's no
buffer, nobody else is going to drop the dqflock, so we need to do it
ourselves.
Cc: stable@vger.kernel.org # v6.13
Fixes: ca378189fd ("xfs: convert quotacheck to attach dquot buffers")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM pointed out a bug in xlog_recover_iunlink_bucket:
1. We don't null out prev_ip after releasing it, which can lead to UAF
problems if the inodegc flush call in the loop fails.
at which point I noticed even more bugs:
2. If the inodegc flush inside the loop fails, we also leak @ip.
3. We set prev_agino to agino having already advanced agino, which
results in inodes with i_prev_unlinked set to itself.
4. If we exit the bottom of the loop with prev_ip set, then prev_ip
aliases ip and we also set its i_prev_unlinked to itself.
Bugs 3 and 4 introduce loops into the unlinked list, though these loops
don't surface because we immediately flush each unlinked inode after
loading it.
Fix all of these issues.
Cc: stable@vger.kernel.org # v6.0
Fixes: 04755d2e58 ("xfs: refactor xlog_recover_process_iunlinks()")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM noticed the copy-paste error here -- if user quotas aren't
enforced but we're near the group quota limit, we fail to set FLAG_GID
and hence we might not actually free any preallocations, causing
unnecessary EDQUOT. Fix that.
Cc: stable@vger.kernel.org # v5.12
Fixes: c237dd7c70 ("xfs: flush eof/cowblocks if we can't reserve quota for inode creation")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM noticed that we can accidentally merge file range health
monitoring events even if they have different errors. We shouldn't do
that.
Cc: stable@vger.kernel.org # v7.0
Fixes: dfa8bad3a8 ("xfs: convey file I/O errors to the health monitor")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM complains that a memory allocation failure in
xrep_newbt_add_blocks results in online repair leaking blocks that were
previously allocated to write a new btree, but the problem is worse than
that -- a limitation of the codebase is that the callers cannot undo the
transaction /and/ return the error -- either you undo all changes and
commit the transaction, or you error out and the filesystem goes down.
However, the new btree space reservation object isn't that big (~48
bytes). Let's just do a NOFAIL allocation and the problem goes away.
Cc: stable@vger.kernel.org # v6.8
Fixes: be40841763 ("xfs: implement block reservation accounting for btrees we're staging")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM points out that runtime errors (e.g. ENOMEM) when we're trying to
compute space usag bitmaps are silently dropped by the rmapbt scrubber.
We ought to flag that as an incomplete scrub instead of reporting
cross-referencing errors based on faulty data.
Cc: stable@vger.kernel.org # v6.4
Fixes: fed050f345 ("xfs: cross-reference rmap records with ag btrees")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
When I added online fsck for realtime reflink, I forgot to update
xrep_calc_rtgroup_resblks to factor in the size of the refcount btree
when it guesses how much space we need to start a repair. This hasn't
been a huge problem in practice because there are few filesystems with
(a) realtime, (b) rtgroups, (c) reflink, and (d) no rmap. But let's fix
this before someone stumbles upon it, especially since LOLLM flagged
this for me.
Cc: stable@vger.kernel.org # v6.14
Fixes: 83ccffc489 ("xfs: online repair of the realtime refcount btree")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
Now that we have quotas for the realtime volume, we also have
precomputed watermark limits for the realtime block counts. These
precomputations should be done any time we change the rtb limits, which
means that xfs_qm_adjust_dqlimits needs to ensure that if we installed
a default rtb limit.
Cc: stable@vger.kernel.org # v6.13
Fixes: 5dd70852b0 ("xfs: create quota preallocation watermarks for realtime quota")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
Correct ten misspellings and eight accidentally doubled words in
comments. No code changes.
The doubled word in xfs_zone_alloc.c was not a duplicate: "so that is is
reused" is "it is" misspelt, so that one reads "so that it is reused"
rather than dropping a word.
Signed-off-by: Hemanth Selam <hemanth.selam@gmail.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
The implementation was inlined into xfs_file_remap_range() by commit
3fc9f5e409 ("xfs: remove xfs_reflink_remap_range"), leaving this
declaration orphaned.
Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
Commit a23eca8844 ("xfs: fix exchange-range reflink flag clearing
issue with INO1_WRITTEN") duplicated commit b2d5a81dae ("xfs: fix
exchange-range reflink flag clearing issue with INO1_WRITTEN"), so
xmi_can_exchange_reflink_flags() ended up with two identical
XFS_EXCHMAPS_INO1_WRITTEN checks. The second one is dead code,
since the first one already returned false. Remove it.
Signed-off-by: Jiangshan Yi <yijiangshan@kylinos.cn>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
oz can be NULL when we resample it after taking i_flags_lock, so account
for that.
Fixes: 2d829cc767 ("xfs: fix racy open zone caching")
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hans Holmberg <hans.holmberg@wdc.com>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
The di_forkoff check in xchk_dinode is incorrect, according to LOLLM.
XFS_DFORK_BOFF returns a byte count relative to the start of the literal
area, not the start of the inode. Therefore, this check won't flag
di_forkoff values that are larger than the literal area but not the
inode size itself. Fix this check; sadly the old APTR code was correct.
Cc: stable@vger.kernel.org # v6.8
Fixes: 6b5d917780 ("xfs: dont cast to char * for XFS_DFORK_*PTR macros")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM complains that xchk_directory_data_bestfree can be passed a
directory block that is either in "block" or "data" format, but the
check here unconditionally treats the dir3_block and dir3_data blocks as
if they have the same header format (they don't). Consequently, we can
incorrectly set the preen state on dir3_block blocks, which of course
we can't preen away because dir3_block blocks do not have a padding
field. Fix this.
Cc: stable@vger.kernel.org # v7.1-rc4
Fixes: 939919ccdd ("xfs: check directory data block header padding in scrub")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM complains that the xbitmap set functions can suffer an integer
underflow or overflow and thereby return the wrong left and right
pointers. Fix that logic bomb, even though (AFAICT) we never actually
try to set the *entire* bitmap.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM noticed that we might reserve the wrong number of blocks for
recovering rtrmap and rtrefcount updates after a crash. Fix that.
Cc: stable@vger.kernel.org # v6.14
Fixes: 5e0679d1c6 ("xfs: support recovering rmap intent items targetting realtime extents")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM noticed that we strip file privileges and whatnot even for a dry
run. We also shouldn't flush dirty data to disk or trim COW staging
events for a dry run. Neither of those behaviors are allowed by the
manpage, so fix that by exiting early on DRY_RUN in various functions.
Cc: stable@vger.kernel.org # v6.10
Fixes: 42672471f9 ("xfs: bind together the front and back ends of the file range exchange code")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM points out that we don't check the ioctl padding field here, so
let's do that. I don't think there are many users yet since exchrange
requires a new feature flag, so it's a good time to try to plug this
hole.
Cc: stable@vger.kernel.org # v6.12
Fixes: 398597c3ef ("xfs: introduce new file range commit ioctls")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM points out that we're supposed to use time_after_eq, not a raw >=
operation here, or else jiffies wraps can go unnoticed. Fix this.
Cc: stable@vger.kernel.org # v6.10
Fixes: 271557de7c ("xfs: reduce the rate of cond_resched calls inside scrub")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM suggests that we not hold the AGFL locked to the scrub transaction
for any longer than we have to when we're rebuilding the rmapbt. Since
we only took it to generate an rmap record for the AGFL blocks, I think
we can safely release it.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM points out that we leak the igrab'd reference to the orphanage
directory inode if chowning it fails. Fix that.
Cc: stable@vger.kernel.org # v6.10
Fixes: 1e58a8ccf2 ("xfs: move orphan files to the orphanage")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM points out that a file has an attr fork, it will call
xchk_inode_count_blocks to set @ablocks to the number of fsblocks mapped
by the attr fork; but then it'll compare @blocks (aka the count of
fsblocks mapped by the data fork). We already checked that and we never
do anything with @acount, so I think this is clearly a bug. Fix the
comparison.
Cc: stable@vger.kernel.org # v6.8
Fixes: 2d295fe657 ("xfs: repair inode records")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
XFS_SCRUB_TYPE_HEALTHY is a synthentic scrub type so that xfs_scrub can
tell the kernel "Hey, I finished a scan and saw no problems" and have
the kernel forget that it saw indirect evidence of corruption.
Unfortunately, as LOLLM points out, it's possible for the health system
to record a new corruption just before xfs_scrub gets to
XFS_SCRUB_TYPE_HEALTHY. In this case, the existing logic doesn't return
early and instead wanders into unknown regions of type_to_health_flag
and trips the assert because HEALTHY doesn't have a group assignment.
Fix the logic so that we always return early for a HEALTHY scrub type,
even if we decide not to call xchk_mark_all_healthy.
Cc: stable@vger.kernel.org # v6.9
Fixes: a1f3e0cca4 ("xfs: update health status if we get a clean bill of health")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM suggests that we need to handle igrab returning NULL here. I
don't think it's possible for the inode to enter I_FREEING or
I_WILL_FREE while we have an active reference to the corresponding
dentry, but we can code defensively anyway.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM pointed out a race condition in xrep_findparent_scan -- the
directory live update hook holds the directory ILOCK when it calls the
xchk_iscan_want_live_update predicate to figure out if it needs to
remember the live update, but xrep_findparent_scan drops the directory
ILOCK before advancing the cursor. Therefore, it's possible for a live
update to check the scan cursor after the scan drops the ILOCK but
before the scan updates its cursor. If this happens, we'll fail to
record the live update. Fix this by moving the cursor update logic
inside xrep_findparent_walk_directory.
Note that for non-directories it's ok to advance the cursor without
holding any ILOCK because the findparent scan only cares about directory
parents, not the children.
Cc: stable@vger.kernel.org # v6.10
Fixes: a07b455762 ("xfs: scan the filesystem to repair a directory dotdot entry")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM noticed that xfs_parent_removename only partially initializes the
passed-in parent pointer arguments object. In the directory tree repair
code, we could decide to remove multiple links to a file, so we don't
want state from one call to bleed into the next one. Zero the whole
thing explicitly.
Cc: stable@vger.kernel.org # v6.10
Fixes: 3f31406aef ("xfs: fix corruptions in the directory tree")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
xrep_dir_replay_removename is the function that replays a directory
entry removal from sc->ip into the temporary directory so that when we
swap the contents of sc->tempip and sc->ip, the directory is correct.
LOLLM noticed that we were passing the wrong inode pointer into
xrep_dir_init_args. It doesn't make sense to set rd->args.dp to
rd->args.dp so let's fix this.
Cc: stable@vger.kernel.org # v6.10
Fixes: 8559b21a64 ("xfs: implement live updates for directory repairs")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
xchk_should_terminate can turn its @error argument into -EINTR if the
user is sitting on ^C. Unfortunately, this code here turns that into a
0 return, which isn't quite correct. LOLLM complains about this, though
I think it's a very minor matter because the only way -EINTR happens is
if there's a fatal signal.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM points out that xchk_bmap_xref_rmap_cow skips the cross-reference
if the data-section rmapbt cursor is not present. However, this is
broken for realtime file data fork scanning, because they will have an
rtrmapbt cursor and not an rmapbt cursor. Fix the behavior by removing
the cursor checks because xchk_bmap_get_rmap already accounts for that.
Cc: stable@vger.kernel.org # v6.14
Fixes: 037a44d827 ("xfs: cross-reference the realtime rmapbt")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM points out that the superblock scrubber doesn't check the new
fields that were added for internal realtime volumes when we added zoned
device support.
Cc: stable@vger.kernel.org # v6.15
Fixes: 2167eaabe2 ("xfs: define the zoned on-disk format")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
Whilst running QA on XFS for-next as of 7.3-rc2 with MKFS_OPTIONS="-n
size=8192", I observed the following (trimmed) dmesg splat:
XFS: Assertion failed: args->total >= dp->i_nblocks - nblks, file: fs/xfs/libxfs/xfs_da_btree.c, line: 2387
WARNING: fs/xfs/xfs_message.c:104 at assfail+0x46/0x4a [xfs], CPU#0: xfs_scrub/1426511
CPU: 0 UID: 0 PID: 1426511 Comm: xfs_scrub Tainted: G W 7.3.0-rc2-djwx #rc2 PREEMPT(lazy) 6e418570b606a39783b0e7e7b30dc407b965f9e8
Tainted: [W]=WARN
RIP: 0010:assfail+0x46/0x4a [xfs]
RSP: 0018:ffffc900010d7890 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 00000000ffffffd1
RDX: 0000000000000000 RSI: 0000000000000021 RDI: ffffffffa059fd38
RBP: 0000000000000002 R08: 0000000000000000 R09: 0000000000000000
R10: 000000000000000a R11: 000000007fffffff R12: ffffc900010d7940
R13: ffff888368d8f980 R14: ffffc900010d7a48 R15: ffffc900010d78d0
FS: 00007f445c5ce680(0000) GS:ffff8884a97ea000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f443803b9a8 CR3: 0000000107a4b000 CR4: 00000000003506f0
Call Trace:
<TASK>
xfs_da_grow_inode_int+0x2e0/0x300 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
xfs_dir2_grow_inode+0x6e/0x150 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
xfs_dir2_sf_to_block+0x149/0x870 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
xrep_dir_swap_prep+0xe2/0x110 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
xrep_dir_swap+0xfb/0x2f0 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
xrep_dir_rebuild_tree+0x99/0x100 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
xrep_directory+0x83/0x1c0 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
xrep_attempt+0x4f/0x1e0 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
xfs_scrub_metadata+0x393/0x5b0 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
xfs_ioc_scrubv_metadata+0x306/0x570 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
xfs_file_ioctl+0xa4f/0x1150 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
__x64_sys_ioctl+0x76/0xc0
do_syscall_64+0x7a/0x3b0
entry_SYSCALL_64_after_hwframe+0x4b/0x53
This is a consequence of commit 0fe77e5758, which added the
following assertion to xfs_da_grow_inode_int:
ASSERT(args->total >= dp->i_nblocks - nblks);
Tracing this back to xrep_dir_swap_prep, I noticed that the xfs_da_args
object that's passed to xfs_dir2_sf_to_block sets args->total to 1.
This is incorrect because mkfs set the directory block size to 8k and
the filesystem block size to 4k. In other words, args->total should be
2 here, not 1.
Dave Chinner tripped over the same problem with the same branch through
a different channel -- his test setup set the fs block size to 1k, in
which case the directory block size is still set to 4k. Here,
args->total should be 4.
Changing the assignment of args->total to sc->mp->m_dir_geo->fsbcount
makes the assertion go away, but that isn't a complete fix. In
xrep_tempexch_estimate, we also incorrectly assume that a shortform
conversion requires 1 fsblock when it should be m_dir_geo->fsbcount.
Without that, we can under-reserve space in the transaction and cause a
filesystem shutdown.
Note that the xfs_dabuf_nfsb helper will compute the correct value for
directories and xattr, so we use that instead of open-coding the logic.
Also fix xrep_xattr_swap_prep to assign args->total via xfs_dabuf_nfsb
to avoid one logic bomb if we ever support multi-fsblock attrs.
Cc: stable@vger.kernel.org # v6.10
Cc: floss@jetm.me
Reported-by: dgc@kernel.org
Fixes: 629fdaf5f5 ("xfs: use atomic extent swapping to fix user file fork data")
Tripped-by: 0fe77e5758 ("xfs: assert the reservation covers each da fork growth")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
__xfs_healthmon_insert() asserts that hm->lock is held
(lockdep_assert_held), but xfs_ioc_health_monitor() called it right
after allocating hm, before ever taking the lock, triggering a
lockdep warning.
Take hm->lock around the call.
Fixes: b3a289a2a9 ("xfs: create event queuing, formatting, and discovery infrastructure")
Reported-by: syzbot+ccdf3469f5f653bff7ac@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ccdf3469f5f653bff7ac
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
xfs_iomap_set_anon_write does not set IOMAP_F_INTEGRITY based on
bdev_has_integrity_csum(), so file system PI generation is silently
skipped for zoned writes on integrity-enabled devices, and left to
the block layer PI generation.
Fixes: 6bbb4d96f7 ("xfs: support T10 protection information")
Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
[hch: ported to the recently introduced xfs_iomap_set_anon_write()]
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
When xlog_force_lsn sets log_flushed, it has just called xlog_force_iclog
through xlog_force_and_check_iclog, which sets XLOG_ICL_NEED_FLUSH before
writing out the head iclog. This means that we already flushed the log,
data, and (with the recent fix) RT devices before writing out the iclog
start record and no extra cache flushed is required.
This optimizes the external log case, and fixes a performance regression
due to double RT dev flushes with "xfs: also flush the RT device cache in
xlog_write_iclog".
The explicit flush of the data that the device resides on when no iclog
was written out is still required.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
As soon as we get an error from cache flushing or log forcing, there
is no point in continuing as the data integrity is already impacted.
Return the error instead of continuing to do more work.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
The cache flush before writing the CIL start record no only needs to
ensure any metadata covered by the overwritten part of the log is on
stable storage, but also that any data pointed to by metadata logged
is on stable storage, as otherwise log recovery could created allocated
blocks that point to stale data. Fortunately the code already
handles this right for the data device, but it also needs to flush
the RT device for this to work for data on the RT device.
Also update the comments to explicitly mention this case.
This omission goes back to the first days of cache control in XFS.
Fixes: 1da177e4c3 ("Linux-2.6.12-rc2")
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM also points out that the xagb_bitmap_set call in xrep_agfl_fill
can fail, but we don't check the result of xagb_bitmap_walk, so we
silently drop the error and proceed with inconsistent incore data.
That shouldn't be allowed.
Cc: stable@vger.kernel.org # v6.6
Fixes: 014ad53732 ("xfs: use per-AG bitmaps to reap unused AG metadata blocks during repair")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM complains that we can't undo an attempt at fixing the AGFL if
anything goes wrong during the rewrite, so take a snapshot of the whole
buffer so that we can restore it. Move the xrep_agfl_update_agf call so
that we only update the AGF if the AGFL update is 100% successful.
While we're at it, fix leaking the used_extents bitmap if the disunion
operation fails.
Cc: stable@vger.kernel.org # v4.19
Fixes: 0e93d3f43e ("xfs: repair the AGFL")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
Remove this useless code.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
Add a new counter so that we can track the number of runtime failures
encountered during scrubs.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM also notices that I forgot to expose the "clean bill of health"
scrub stats. Fix that.
Cc: stable@vger.kernel.org # v6.9
Fixes: a1f3e0cca4 ("xfs: update health status if we get a clean bill of health")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM complains about concurrency problems in the scrub stats code
because xchk_stats_format doesn't synchronize in any way with updates.
These stats are only reported through debugfs so I don't think it really
matters, but I guess I exist to make bots happy now.
Note: We snapshot the entire stats object with a spinlock so that we
don't have to worry about users seeing slightly weird numbers (e.g.
invocations has incremented but none of the outcomes have been yet) if
we race with xchk_stats_merge_one. This isn't a hot path.
Cc: stable@vger.kernel.org # v6.6
Fixes: d7a74cad8f ("xfs: track usage statistics of online fsck")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
Over time a number of function declarations in various headers have become
stale: either their implementations were removed when their last callers
went away, or they were never implemented in the first place. None of them
refer to anything anymore.
Remove the following dead declarations and the unused stub:
- xlog_assign_tail_lsn() and xlog_assign_tail_lsn_locked()
- xfs_iext_realloc()
- xfs_buf_iodone()
- xfs_scrub_tester() and xfs_scrub_setup_inode_bmap_data() (never
implemented placeholders)
- the !CONFIG_XFS_ONLINE_REPAIR stub of xrep_tempfile_iolock_both()
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
The main scrub code will quietly turn bubbled-up EFSCORRUPTED and
EFSBADCRC errors into corruption errors. These aren't recorded in the
scrub stats code (says LOLLM) so do that now.
Cc: stable@vger.kernel.org # v6.6
Fixes: d7a74cad8f ("xfs: track usage statistics of online fsck")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM noticed that the rtsummary repair code will create a new rtsummary
with the correct file size, but it won't force the new file size to be
set on the existing rtsummary file, leaving the rtsummary corrupt. Fix
this by setting up the tempfile mapping-exchange to run to the end of
both files, which is the magic offset needed to reset the file size.
Cc: stable@vger.kernel.org # v6.10
Fixes: abf039e2e4 ("xfs: online repair of realtime summaries")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM started on a long tangent about how xchk_refcount_xref_rmap
shouldn't nope out if sc->sa.rmap_cur isn't set, because nothing ever
sets that field. It's right about the condition, but misses the bigger
problem, which is that to count the rtrefcount btree blocks, we have to
walk all rmap records in each AG in the data section. That was papered
over by the incorrect !sc->sa.rmap_cur test.
In other words, we need a perag iteration loop here. Restructure the
code to do that, and now it'll all work properly. Fix the confusing
function name prefix.
Cc: stable@vger.kernel.org # v6.14
Fixes: c27929670d ("xfs: scrub the realtime refcount btree")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM pointed out a bug in both of the refcount scrub predicates that
determine if a range of blocks is marked as CoW staging in the btree.
While it compares blockcount < len, this isn't enough to determine that
the CoW staging record is at least as large as the range passed into the
helper. Fix both of them.
Cc: stable@vger.kernel.org # v4.16
Fixes: f6d5fc21fd ("xfs: cross-reference refcount btree during scrub")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
LOLLM noticed an anachronism from the early days of rtrefcount where the
refcount btree would handle 64-bit block numbers -- we pass rtblocks
into the gap finder, but rtrefcount btrees are sharded by rtgroup now.
This isn't really a problem for us since we're only looking for
overlapping rtrmap records to flag, but let's fix this sillyness.
Also fix some stale comments.
Cc: stable@vger.kernel.org # v6.14
Fixes: 30f47950dc ("xfs: check reference counts of gaps between rt refcount records")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
