Skip to content

Commit 27c14b5

Browse files
committed
xfs: ensure inobt record walks always make forward progress
The aim of the inode btree record iterator function is to call a callback on every record in the btree. To avoid having to tear down and recreate the inode btree cursor around every callback, it caches a certain number of records in a memory buffer. After each batch of callback invocations, we have to perform a btree lookup to find the next record after where we left off. However, if the keys of the inode btree are corrupt, the lookup might put us in the wrong part of the inode btree, causing the walk function to loop forever. Therefore, we add extra cursor tracking to make sure that we never go backwards neither when performing the lookup nor when jumping to the next inobt record. This also fixes an off by one error where upon resume the lookup should have been for the inode /after/ the point at which we stopped. Found by fuzzing xfs/460 with keys[2].startino = ones causing bulkstat and quotacheck to hang. Fixes: a211432 ("xfs: create simplified inode walk function") Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
1 parent ada49d6 commit 27c14b5

File tree

1 file changed

+24
-3
lines changed

1 file changed

+24
-3
lines changed

fs/xfs/xfs_iwalk.c

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@ struct xfs_iwalk_ag {
5555
/* Where do we start the traversal? */
5656
xfs_ino_t startino;
5757

58+
/* What was the last inode number we saw when iterating the inobt? */
59+
xfs_ino_t lastino;
60+
5861
/* Array of inobt records we cache. */
5962
struct xfs_inobt_rec_incore *recs;
6063

@@ -301,6 +304,9 @@ xfs_iwalk_ag_start(
301304
if (XFS_IS_CORRUPT(mp, *has_more != 1))
302305
return -EFSCORRUPTED;
303306

307+
iwag->lastino = XFS_AGINO_TO_INO(mp, agno,
308+
irec->ir_startino + XFS_INODES_PER_CHUNK - 1);
309+
304310
/*
305311
* If the LE lookup yielded an inobt record before the cursor position,
306312
* skip it and see if there's another one after it.
@@ -347,15 +353,17 @@ xfs_iwalk_run_callbacks(
347353
struct xfs_mount *mp = iwag->mp;
348354
struct xfs_trans *tp = iwag->tp;
349355
struct xfs_inobt_rec_incore *irec;
350-
xfs_agino_t restart;
356+
xfs_agino_t next_agino;
351357
int error;
352358

359+
next_agino = XFS_INO_TO_AGINO(mp, iwag->lastino) + 1;
360+
353361
ASSERT(iwag->nr_recs > 0);
354362

355363
/* Delete cursor but remember the last record we cached... */
356364
xfs_iwalk_del_inobt(tp, curpp, agi_bpp, 0);
357365
irec = &iwag->recs[iwag->nr_recs - 1];
358-
restart = irec->ir_startino + XFS_INODES_PER_CHUNK - 1;
366+
ASSERT(next_agino == irec->ir_startino + XFS_INODES_PER_CHUNK);
359367

360368
error = xfs_iwalk_ag_recs(iwag);
361369
if (error)
@@ -372,7 +380,7 @@ xfs_iwalk_run_callbacks(
372380
if (error)
373381
return error;
374382

375-
return xfs_inobt_lookup(*curpp, restart, XFS_LOOKUP_GE, has_more);
383+
return xfs_inobt_lookup(*curpp, next_agino, XFS_LOOKUP_GE, has_more);
376384
}
377385

378386
/* Walk all inodes in a single AG, from @iwag->startino to the end of the AG. */
@@ -396,6 +404,7 @@ xfs_iwalk_ag(
396404

397405
while (!error && has_more) {
398406
struct xfs_inobt_rec_incore *irec;
407+
xfs_ino_t rec_fsino;
399408

400409
cond_resched();
401410
if (xfs_pwork_want_abort(&iwag->pwork))
@@ -407,6 +416,15 @@ xfs_iwalk_ag(
407416
if (error || !has_more)
408417
break;
409418

419+
/* Make sure that we always move forward. */
420+
rec_fsino = XFS_AGINO_TO_INO(mp, agno, irec->ir_startino);
421+
if (iwag->lastino != NULLFSINO &&
422+
XFS_IS_CORRUPT(mp, iwag->lastino >= rec_fsino)) {
423+
error = -EFSCORRUPTED;
424+
goto out;
425+
}
426+
iwag->lastino = rec_fsino + XFS_INODES_PER_CHUNK - 1;
427+
410428
/* No allocated inodes in this chunk; skip it. */
411429
if (iwag->skip_empty && irec->ir_freecount == irec->ir_count) {
412430
error = xfs_btree_increment(cur, 0, &has_more);
@@ -535,6 +553,7 @@ xfs_iwalk(
535553
.trim_start = 1,
536554
.skip_empty = 1,
537555
.pwork = XFS_PWORK_SINGLE_THREADED,
556+
.lastino = NULLFSINO,
538557
};
539558
xfs_agnumber_t agno = XFS_INO_TO_AGNO(mp, startino);
540559
int error;
@@ -623,6 +642,7 @@ xfs_iwalk_threaded(
623642
iwag->data = data;
624643
iwag->startino = startino;
625644
iwag->sz_recs = xfs_iwalk_prefetch(inode_records);
645+
iwag->lastino = NULLFSINO;
626646
xfs_pwork_queue(&pctl, &iwag->pwork);
627647
startino = XFS_AGINO_TO_INO(mp, agno + 1, 0);
628648
if (flags & XFS_INOBT_WALK_SAME_AG)
@@ -696,6 +716,7 @@ xfs_inobt_walk(
696716
.startino = startino,
697717
.sz_recs = xfs_inobt_walk_prefetch(inobt_records),
698718
.pwork = XFS_PWORK_SINGLE_THREADED,
719+
.lastino = NULLFSINO,
699720
};
700721
xfs_agnumber_t agno = XFS_INO_TO_AGNO(mp, startino);
701722
int error;

0 commit comments

Comments
 (0)