mirror of
https://github.com/torvalds/linux.git
synced 2026-09-23 13:14:02 +02:00
xfs: fix cursor and pointer handling when recovering iunlink buckets
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>
This commit is contained in:
parent
f8f6382ff1
commit
65f39d09d7
|
|
@ -2736,12 +2736,13 @@ xlog_recover_iunlink_bucket(
|
|||
{
|
||||
struct xfs_mount *mp = pag_mount(pag);
|
||||
struct xfs_inode *prev_ip = NULL;
|
||||
struct xfs_inode *ip;
|
||||
xfs_agino_t prev_agino, agino;
|
||||
int error = 0;
|
||||
|
||||
agino = be32_to_cpu(agi->agi_unlinked[bucket]);
|
||||
while (agino != NULLAGINO) {
|
||||
struct xfs_inode *ip;
|
||||
|
||||
error = xfs_iget(mp, NULL, xfs_agino_to_ino(pag, agino), 0, 0,
|
||||
&ip);
|
||||
if (error)
|
||||
|
|
@ -2750,11 +2751,11 @@ xlog_recover_iunlink_bucket(
|
|||
ASSERT(VFS_I(ip)->i_nlink == 0);
|
||||
ASSERT(VFS_I(ip)->i_mode != 0);
|
||||
xfs_iflags_clear(ip, XFS_IRECOVERY);
|
||||
agino = ip->i_next_unlinked;
|
||||
|
||||
if (prev_ip) {
|
||||
ip->i_prev_unlinked = prev_agino;
|
||||
xfs_irele(prev_ip);
|
||||
prev_ip = NULL;
|
||||
|
||||
/*
|
||||
* Ensure the inode is removed from the unlinked list
|
||||
|
|
@ -2766,18 +2767,20 @@ xlog_recover_iunlink_bucket(
|
|||
* complete.
|
||||
*/
|
||||
error = xfs_inodegc_flush(mp);
|
||||
if (error)
|
||||
break;
|
||||
if (error) {
|
||||
xfs_irele(ip);
|
||||
return error;
|
||||
}
|
||||
}
|
||||
|
||||
prev_agino = agino;
|
||||
agino = ip->i_next_unlinked;
|
||||
prev_ip = ip;
|
||||
}
|
||||
|
||||
if (prev_ip) {
|
||||
int error2;
|
||||
|
||||
ip->i_prev_unlinked = prev_agino;
|
||||
xfs_irele(prev_ip);
|
||||
|
||||
error2 = xfs_inodegc_flush(mp);
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user