From: Dave Chinner <dgc@kernel.org>
To: linux-xfs@vger.kernel.org
Cc: cem@kernel.org
Subject: [PATCH 19/48] xfs: validate extent free log items during recovery
Date: Tue, 13 Oct 2026 17:01:09 +1100 [thread overview]
Message-ID: <20261013060204.3138690-20-dgc@kernel.org> (raw)
In-Reply-To: <20261013060204.3138690-1-dgc@kernel.org>
The extent free intent and done items are single region items whose length
depends on the extent count declared in the region. Recovery only checked this
in the commit handlers, after the item had already been decoded and
initialised. Validate the structure up front as the region is decoded instead.
The data and realtime variants share these checks as their formats are
identical.
The commit handlers and xfs_efi_copy_format() can now trust the structure, so
the redundant checks are removed from them. xfs_efi_copy_format() no longer has
a failure case and returns void.
Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
fs/xfs/xfs_extfree_item.c | 217 ++++++++++++++++++++++++--------------
1 file changed, 136 insertions(+), 81 deletions(-)
diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
index 2266d56e37dc..c8d15de582f5 100644
--- a/fs/xfs/xfs_extfree_item.c
+++ b/fs/xfs/xfs_extfree_item.c
@@ -180,58 +180,58 @@ xfs_efi_init(
* one of which will be the native format for this kernel.
* It will handle the conversion of formats if necessary.
*/
-STATIC int
+STATIC void
xfs_efi_copy_format(
struct kvec *buf,
struct xfs_efi_log_format *dst_efi_fmt)
{
struct xfs_efi_log_format *src_efi_fmt = buf->iov_base;
- uint len, len32, len64, i;
-
- len = xfs_efi_log_format_sizeof(src_efi_fmt->efi_nextents);
- len32 = xfs_efi_log_format32_sizeof(src_efi_fmt->efi_nextents);
- len64 = xfs_efi_log_format64_sizeof(src_efi_fmt->efi_nextents);
+ struct xfs_efi_log_format_32 *src_efi_fmt_32;
+ struct xfs_efi_log_format_64 *src_efi_fmt_64;
+ uint nextents = src_efi_fmt->efi_nextents;
+ uint i;
- if (buf->iov_len == len) {
+ /*
+ * The buffer has already been validated as one of the three formats, so
+ * pick which one it is by its length and copy it into the native form.
+ */
+ if (buf->iov_len == xfs_efi_log_format_sizeof(nextents)) {
memcpy(dst_efi_fmt, src_efi_fmt,
offsetof(struct xfs_efi_log_format, efi_extents));
- for (i = 0; i < src_efi_fmt->efi_nextents; i++)
+ for (i = 0; i < nextents; i++)
memcpy(&dst_efi_fmt->efi_extents[i],
&src_efi_fmt->efi_extents[i],
sizeof(struct xfs_extent));
- return 0;
- } else if (buf->iov_len == len32) {
- struct xfs_efi_log_format_32 *src_efi_fmt_32 = buf->iov_base;
+ return;
+ }
+ if (buf->iov_len == xfs_efi_log_format32_sizeof(nextents)) {
+ src_efi_fmt_32 = buf->iov_base;
dst_efi_fmt->efi_type = src_efi_fmt_32->efi_type;
dst_efi_fmt->efi_size = src_efi_fmt_32->efi_size;
dst_efi_fmt->efi_nextents = src_efi_fmt_32->efi_nextents;
dst_efi_fmt->efi_id = src_efi_fmt_32->efi_id;
- for (i = 0; i < dst_efi_fmt->efi_nextents; i++) {
+ for (i = 0; i < nextents; i++) {
dst_efi_fmt->efi_extents[i].ext_start =
src_efi_fmt_32->efi_extents[i].ext_start;
dst_efi_fmt->efi_extents[i].ext_len =
src_efi_fmt_32->efi_extents[i].ext_len;
}
- return 0;
- } else if (buf->iov_len == len64) {
- struct xfs_efi_log_format_64 *src_efi_fmt_64 = buf->iov_base;
-
- dst_efi_fmt->efi_type = src_efi_fmt_64->efi_type;
- dst_efi_fmt->efi_size = src_efi_fmt_64->efi_size;
- dst_efi_fmt->efi_nextents = src_efi_fmt_64->efi_nextents;
- dst_efi_fmt->efi_id = src_efi_fmt_64->efi_id;
- for (i = 0; i < dst_efi_fmt->efi_nextents; i++) {
- dst_efi_fmt->efi_extents[i].ext_start =
- src_efi_fmt_64->efi_extents[i].ext_start;
- dst_efi_fmt->efi_extents[i].ext_len =
- src_efi_fmt_64->efi_extents[i].ext_len;
- }
- return 0;
+ return;
+ }
+
+ ASSERT(buf->iov_len == xfs_efi_log_format64_sizeof(nextents));
+ src_efi_fmt_64 = buf->iov_base;
+ dst_efi_fmt->efi_type = src_efi_fmt_64->efi_type;
+ dst_efi_fmt->efi_size = src_efi_fmt_64->efi_size;
+ dst_efi_fmt->efi_nextents = src_efi_fmt_64->efi_nextents;
+ dst_efi_fmt->efi_id = src_efi_fmt_64->efi_id;
+ for (i = 0; i < nextents; i++) {
+ dst_efi_fmt->efi_extents[i].ext_start =
+ src_efi_fmt_64->efi_extents[i].ext_start;
+ dst_efi_fmt->efi_extents[i].ext_len =
+ src_efi_fmt_64->efi_extents[i].ext_len;
}
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, NULL, buf->iov_base,
- buf->iov_len);
- return -EFSCORRUPTED;
}
static inline struct xfs_efd_log_item *EFD_ITEM(struct xfs_log_item *lip)
@@ -864,22 +864,11 @@ xlog_recover_efi_commit_pass2(
struct xfs_mount *mp = log->l_mp;
struct xfs_efi_log_item *efip;
struct xfs_efi_log_format *efi_formatp;
- int error;
efi_formatp = item->ri_buf[0].iov_base;
- if (item->ri_buf[0].iov_len < xfs_efi_log_format_sizeof(0)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
- item->ri_buf[0].iov_base, item->ri_buf[0].iov_len);
- return -EFSCORRUPTED;
- }
-
efip = xfs_efi_init(mp, ITEM_TYPE(item), efi_formatp->efi_nextents);
- error = xfs_efi_copy_format(&item->ri_buf[0], &efip->efi_format);
- if (error) {
- xfs_efi_item_free(efip);
- return error;
- }
+ xfs_efi_copy_format(&item->ri_buf[0], &efip->efi_format);
atomic_set(&efip->efi_next_extent, efi_formatp->efi_nextents);
xlog_recover_intent_item(log, &efip->efi_item, lsn,
@@ -887,8 +876,60 @@ xlog_recover_efi_commit_pass2(
return 0;
}
+/*
+ * The efi is a single region whose length must match the extent count for one
+ * of the native, 32 bit or 64 bit on-disk formats. This is shared by the data
+ * and realtime efi items, which have identical formats.
+ */
+static int
+xlog_recover_efi_validate_region(
+ struct xlog *log,
+ struct xlog_recover_item *item,
+ int region_index)
+{
+ struct xfs_efi_log_format *efi_formatp =
+ item->ri_buf[region_index].iov_base;
+ size_t len = item->ri_buf[region_index].iov_len;
+ uint32_t nextents;
+
+ if (len < xfs_efi_log_format_sizeof(0)) {
+ xfs_warn(log->l_mp, "EFI: bad item region length %zu recovered.",
+ len);
+ return -EFSCORRUPTED;
+ }
+
+ /*
+ * Bound the extent count before the exact size checks below so a corrupt
+ * count cannot overflow the size calculations. This limit is a fuzzy
+ * upper bound using the smallest extent encoding to prevent overflow; it
+ * does not need to be exact as the size checks that follow enforce the
+ * precise length for each format.
+ */
+ nextents = efi_formatp->efi_nextents;
+ if (nextents == 0 ||
+ nextents > XLOG_MAX_REGION_SIZE / sizeof(struct xfs_extent_32)) {
+ xfs_warn(log->l_mp, "EFI: bad extent count %u recovered.",
+ nextents);
+ return -EFSCORRUPTED;
+ }
+
+ if (len != xfs_efi_log_format_sizeof(nextents) &&
+ len != xfs_efi_log_format32_sizeof(nextents) &&
+ len != xfs_efi_log_format64_sizeof(nextents)) {
+ xfs_warn(log->l_mp,
+ "EFI: item region length %zu does not match %u extents recovered.",
+ len, nextents);
+ return -EFSCORRUPTED;
+ }
+
+ return 0;
+}
+
const struct xlog_recover_item_ops xlog_efi_item_ops = {
.item_type = XFS_LI_EFI,
+ .min_hdr_len = sizeof(struct xfs_efi_log_format),
+ .validate_nregions = xlog_recover_single_region,
+ .validate_region = xlog_recover_efi_validate_region,
.commit_pass2 = xlog_recover_efi_commit_pass2,
};
@@ -903,22 +944,11 @@ xlog_recover_rtefi_commit_pass2(
struct xfs_mount *mp = log->l_mp;
struct xfs_efi_log_item *efip;
struct xfs_efi_log_format *efi_formatp;
- int error;
efi_formatp = item->ri_buf[0].iov_base;
- if (item->ri_buf[0].iov_len < xfs_efi_log_format_sizeof(0)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
- item->ri_buf[0].iov_base, item->ri_buf[0].iov_len);
- return -EFSCORRUPTED;
- }
-
efip = xfs_efi_init(mp, ITEM_TYPE(item), efi_formatp->efi_nextents);
- error = xfs_efi_copy_format(&item->ri_buf[0], &efip->efi_format);
- if (error) {
- xfs_efi_item_free(efip);
- return error;
- }
+ xfs_efi_copy_format(&item->ri_buf[0], &efip->efi_format);
atomic_set(&efip->efi_next_extent, efi_formatp->efi_nextents);
xlog_recover_intent_item(log, &efip->efi_item, lsn,
@@ -941,6 +971,9 @@ xlog_recover_rtefi_commit_pass2(
const struct xlog_recover_item_ops xlog_rtefi_item_ops = {
.item_type = XFS_LI_EFI_RT,
+ .min_hdr_len = sizeof(struct xfs_efi_log_format),
+ .validate_nregions = xlog_recover_single_region,
+ .validate_region = xlog_recover_efi_validate_region,
.commit_pass2 = xlog_recover_rtefi_commit_pass2,
};
@@ -959,31 +992,66 @@ xlog_recover_efd_commit_pass2(
xfs_lsn_t lsn)
{
struct xfs_efd_log_format *efd_formatp;
- int buflen = item->ri_buf[0].iov_len;
efd_formatp = item->ri_buf[0].iov_base;
- if (buflen < sizeof(struct xfs_efd_log_format)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, log->l_mp,
- efd_formatp, buflen);
+ xlog_recover_release_intent(log, XFS_LI_EFI, efd_formatp->efd_efi_id);
+ return 0;
+}
+
+/*
+ * The efd is a single region whose length must match the extent count for the
+ * 32 bit or 64 bit on-disk format. This is shared by the data and realtime efd
+ * items, which have identical formats.
+ */
+static int
+xlog_recover_efd_validate_region(
+ struct xlog *log,
+ struct xlog_recover_item *item,
+ int region_index)
+{
+ struct xfs_efd_log_format *efd_formatp =
+ item->ri_buf[region_index].iov_base;
+ size_t len = item->ri_buf[region_index].iov_len;
+ uint32_t nextents;
+
+ if (len < sizeof(struct xfs_efd_log_format)) {
+ xfs_warn(log->l_mp, "EFD: bad item region length %zu recovered.",
+ len);
return -EFSCORRUPTED;
}
- if (item->ri_buf[0].iov_len != xfs_efd_log_format32_sizeof(
- efd_formatp->efd_nextents) &&
- item->ri_buf[0].iov_len != xfs_efd_log_format64_sizeof(
- efd_formatp->efd_nextents)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, log->l_mp,
- efd_formatp, buflen);
+ /*
+ * Bound the extent count before the exact size checks below so a corrupt
+ * count cannot overflow the size calculations. This limit is a fuzzy
+ * upper bound using the smallest extent encoding to prevent overflow; it
+ * does not need to be exact as the size checks that follow enforce the
+ * precise length for each format.
+ */
+ nextents = efd_formatp->efd_nextents;
+ if (nextents == 0 ||
+ nextents > XLOG_MAX_REGION_SIZE / sizeof(struct xfs_extent_32)) {
+ xfs_warn(log->l_mp, "EFD: bad extent count %u recovered.",
+ nextents);
+ return -EFSCORRUPTED;
+ }
+
+ if (len != xfs_efd_log_format32_sizeof(nextents) &&
+ len != xfs_efd_log_format64_sizeof(nextents)) {
+ xfs_warn(log->l_mp,
+ "EFD: item region length %zu does not match %u extents recovered.",
+ len, nextents);
return -EFSCORRUPTED;
}
- xlog_recover_release_intent(log, XFS_LI_EFI, efd_formatp->efd_efi_id);
return 0;
}
const struct xlog_recover_item_ops xlog_efd_item_ops = {
.item_type = XFS_LI_EFD,
+ .min_hdr_len = sizeof(struct xfs_efd_log_format),
+ .validate_nregions = xlog_recover_single_region,
+ .validate_region = xlog_recover_efd_validate_region,
.commit_pass2 = xlog_recover_efd_commit_pass2,
};
@@ -996,25 +1064,9 @@ xlog_recover_rtefd_commit_pass2(
xfs_lsn_t lsn)
{
struct xfs_efd_log_format *efd_formatp;
- int buflen = item->ri_buf[0].iov_len;
efd_formatp = item->ri_buf[0].iov_base;
- if (buflen < sizeof(struct xfs_efd_log_format)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, log->l_mp,
- efd_formatp, buflen);
- return -EFSCORRUPTED;
- }
-
- if (item->ri_buf[0].iov_len != xfs_efd_log_format32_sizeof(
- efd_formatp->efd_nextents) &&
- item->ri_buf[0].iov_len != xfs_efd_log_format64_sizeof(
- efd_formatp->efd_nextents)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, log->l_mp,
- efd_formatp, buflen);
- return -EFSCORRUPTED;
- }
-
xlog_recover_release_intent(log, XFS_LI_EFI_RT,
efd_formatp->efd_efi_id);
return 0;
@@ -1025,5 +1077,8 @@ xlog_recover_rtefd_commit_pass2(
const struct xlog_recover_item_ops xlog_rtefd_item_ops = {
.item_type = XFS_LI_EFD_RT,
+ .min_hdr_len = sizeof(struct xfs_efd_log_format),
+ .validate_nregions = xlog_recover_single_region,
+ .validate_region = xlog_recover_efd_validate_region,
.commit_pass2 = xlog_recover_rtefd_commit_pass2,
};
--
2.55.0
next prev parent reply other threads:[~2026-10-13 6:02 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-13 6:00 [RFC] [PATCH 00/48] Validate journal log items during log recovery Dave Chinner
2026-10-13 6:00 ` [PATCH 01/48] xfs: set the unmount record operation header length Dave Chinner
2026-10-13 6:00 ` [PATCH 02/48] xfs: add log recovery item validation design document Dave Chinner
2026-10-13 6:00 ` [PATCH 03/48] xfs: lift transaction header parsing out of the region assembly Dave Chinner
2026-10-13 6:00 ` [PATCH 04/48] xfs: track transaction header decode state explicitly Dave Chinner
2026-10-13 6:00 ` [PATCH 05/48] xfs: track the log item being rebuilt with an explicit pointer Dave Chinner
2026-10-13 6:00 ` [PATCH 06/48] xfs: complete log items explicitly during recovery Dave Chinner
2026-10-13 6:00 ` [PATCH 07/48] xfs: add log item validation infrastructure to log recovery Dave Chinner
2026-10-13 6:00 ` [PATCH 08/48] xfs: recover the transaction header as a validated item type Dave Chinner
2026-10-13 6:00 ` [PATCH 09/48] xfs: factor out xlog_recover_validate_ophdr() Dave Chinner
2026-10-13 6:01 ` [PATCH 10/48] xfs: validate operation header length and alignment during recovery Dave Chinner
2026-10-13 6:01 ` [PATCH 11/48] xfs: validate the operation header padding " Dave Chinner
2026-10-13 6:01 ` [PATCH 12/48] xfs: validate the operation header flags " Dave Chinner
2026-10-13 6:01 ` [PATCH 13/48] xfs: enforce zero length control records " Dave Chinner
2026-10-13 6:01 ` [PATCH 14/48] xfs: validate the unmount record " Dave Chinner
2026-10-13 6:01 ` [PATCH 15/48] xfs: handle zero length continuation op headers " Dave Chinner
2026-10-13 6:01 ` [PATCH 16/48] xfs: bound the assembled region size " Dave Chinner
2026-10-13 6:01 ` [PATCH 17/48] xfs: dump the corrupt region when log item validation fails Dave Chinner
2026-10-13 6:01 ` [PATCH 18/48] xfs: validate fixed size log items during recovery Dave Chinner
2026-10-13 6:01 ` Dave Chinner [this message]
2026-10-13 6:01 ` [PATCH 20/48] xfs: validate and convert extent free intents " Dave Chinner
2026-10-13 6:01 ` [PATCH 21/48] xfs: validate and convert extent free done items " Dave Chinner
2026-10-13 6:01 ` [PATCH 22/48] xfs: validate rmap update intents " Dave Chinner
2026-10-13 6:01 ` [PATCH 23/48] xfs: validate rmap update done items " Dave Chinner
2026-10-13 6:01 ` [PATCH 24/48] xfs: validate refcount update intents " Dave Chinner
2026-10-13 6:01 ` [PATCH 25/48] xfs: validate refcount update done items " Dave Chinner
2026-10-13 6:01 ` [PATCH 26/48] xfs: validate bmap update intents " Dave Chinner
2026-10-13 6:01 ` [PATCH 27/48] xfs: validate bmap update done items " Dave Chinner
2026-10-13 6:01 ` [PATCH 28/48] xfs: validate mapping exchange intents " Dave Chinner
2026-10-13 6:01 ` [PATCH 29/48] xfs: validate mapping exchange done items " Dave Chinner
2026-10-13 6:01 ` [PATCH 30/48] xfs: validate dquot " Dave Chinner
2026-10-13 6:01 ` [PATCH 31/48] xfs: validate attr done " Dave Chinner
2026-10-13 6:01 ` [PATCH 32/48] xfs: validate attr intents " Dave Chinner
2026-10-13 6:01 ` [PATCH 33/48] xfs: convert inode items to host format " Dave Chinner
2026-10-13 6:01 ` [PATCH 34/48] xfs: allow xfs_imap() to map without an inode btree lookup Dave Chinner
2026-10-13 6:01 ` [PATCH 35/48] xfs: validate the inode item format header during recovery Dave Chinner
2026-10-13 6:01 ` [PATCH 36/48] xfs: validate the logged inode core " Dave Chinner
2026-10-13 6:01 ` [PATCH 37/48] xfs: validate the logged inode forks " Dave Chinner
2026-10-13 6:01 ` [PATCH 38/48] xfs: validate logged local fork contents " Dave Chinner
2026-10-13 6:01 ` [PATCH 39/48] xfs: validate logged extent " Dave Chinner
2026-10-13 6:01 ` [PATCH 40/48] xfs: allow a long format btree root to be checked without a cursor Dave Chinner
2026-10-13 6:01 ` [PATCH 41/48] xfs: validate logged btree fork roots during recovery Dave Chinner
2026-10-13 6:01 ` [PATCH 42/48] xfs: validate logged metadata " Dave Chinner
2026-10-13 6:01 ` [PATCH 43/48] xfs: validate buffer item region count " Dave Chinner
2026-10-13 6:01 ` [PATCH 44/48] xfs: validate buffer item regions " Dave Chinner
2026-10-13 6:01 ` [PATCH 45/48] xfs: validate the whole buffer item " Dave Chinner
2026-10-13 6:01 ` [PATCH 46/48] xfs: remove redundant buffer checks from recovery replay Dave Chinner
2026-10-13 6:01 ` [PATCH 47/48] xfs: fail recovery on a buffer type mismatch Dave Chinner
2026-10-13 6:01 ` [PATCH 48/48] xfs: validate logged dquot buffers during recovery Dave Chinner
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=20261013060204.3138690-20-dgc@kernel.org \
--to=dgc@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