From: "Darrick J. Wong" <djwong@kernel.org>
To: Carlos Maiolino <cem@kernel.org>
Cc: xfs <linux-xfs@vger.kernel.org>
Subject: [PATCH] xfs_repair: dont leak buffer when discarding directories
Date: Wed, 3 May 2023 08:15:15 -0700 [thread overview]
Message-ID: <20230503151515.GD15394@frogsfrogsfrogs> (raw)
From: Darrick J. Wong <djwong@kernel.org>
Commit 1f7c7553489c tried to reduce the memory requirements of phase 6
of repair by redesigning longform_dir2_entry_check without the bplist
array. Unfortunately, none of us noticed that the code that rejects a
dir block with a bad header now leaks the xfs_buf object because we no
longer have a bplist to drop the buffer references. Any time we hold a
buffer and decide to move on in the dabno loop, we must release the
buffer.
The immediate result of this error is that dir_binval complains about
the recursive lock count of the buffer when we blow out the directory.
However, if the block is reallocated by another thread, repair will
deadlock when it tries to get the buffer and cannot take the buffer
lock.
Found via xfs/113 fuzzing data format directory blocks. For whatever
reason this happens much more frequently when su=128k,sw=4, but this
applies to everyone equally.
While we're at it, make the relse at the bottom of the function run for
any remaining buffer reference, even if this isn't a block format
directory to avoid leaving a landmine in case we ever add a "goto
fix" inside the loop for a non-block directory.
Fixes: 1f7c7553489 ("repair: don't duplicate names in phase 6")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
repair/phase6.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/repair/phase6.c b/repair/phase6.c
index 0be2c9c9705..48bf57359c5 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -2332,6 +2332,9 @@ longform_dir2_entry_check(
fixit++;
if (isblock)
goto out_fix;
+
+ libxfs_buf_relse(bp);
+ bp = NULL;
continue;
}
}
@@ -2343,6 +2346,7 @@ longform_dir2_entry_check(
break;
libxfs_buf_relse(bp);
+ bp = NULL;
}
fixit |= (*num_illegal != 0) || dir2_is_badino(ino) || *need_dot;
@@ -2370,7 +2374,7 @@ longform_dir2_entry_check(
}
}
out_fix:
- if (isblock && bp)
+ if (bp)
libxfs_buf_relse(bp);
if (!no_modify && (fixit || dotdot_update)) {
next reply other threads:[~2023-05-03 15:15 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <hisABZGtTerz9q4LHi-k52Q9qnsVnsnnpL0ZyXEQleLbIaF5zCFcA_URJ65VWBQpaCD_1oSQW9iBbmheoPZ8TA==@protonmail.internalid>
2023-05-03 15:15 ` Darrick J. Wong [this message]
2023-05-10 14:00 ` [PATCH] xfs_repair: dont leak buffer when discarding directories Carlos Maiolino
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230503151515.GD15394@frogsfrogsfrogs \
--to=djwong@kernel.org \
--cc=cem@kernel.org \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).