From: Catherine Hoang <catherine.hoang@oracle.com>
To: linux-xfs@vger.kernel.org
Subject: [PATCH 6.6 CANDIDATE 08/18] xfs: don't walk off the end of a directory data block
Date: Tue, 17 Dec 2024 18:14:01 -0800 [thread overview]
Message-ID: <20241218021411.42144-9-catherine.hoang@oracle.com> (raw)
In-Reply-To: <20241218021411.42144-1-catherine.hoang@oracle.com>
From: lei lu <llfamsec@gmail.com>
commit 0c7fcdb6d06cdf8b19b57c17605215b06afa864a upstream.
This adds sanity checks for xfs_dir2_data_unused and xfs_dir2_data_entry
to make sure don't stray beyond valid memory region. Before patching, the
loop simply checks that the start offset of the dup and dep is within the
range. So in a crafted image, if last entry is xfs_dir2_data_unused, we
can change dup->length to dup->length-1 and leave 1 byte of space. In the
next traversal, this space will be considered as dup or dep. We may
encounter an out of bound read when accessing the fixed members.
In the patch, we make sure that the remaining bytes large enough to hold
an unused entry before accessing xfs_dir2_data_unused and
xfs_dir2_data_unused is XFS_DIR2_DATA_ALIGN byte aligned. We also make
sure that the remaining bytes large enough to hold a dirent with a
single-byte name before accessing xfs_dir2_data_entry.
Signed-off-by: lei lu <llfamsec@gmail.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
---
fs/xfs/libxfs/xfs_dir2_data.c | 31 ++++++++++++++++++++++++++-----
fs/xfs/libxfs/xfs_dir2_priv.h | 7 +++++++
2 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
index dbcf58979a59..e1d5da6d8d4a 100644
--- a/fs/xfs/libxfs/xfs_dir2_data.c
+++ b/fs/xfs/libxfs/xfs_dir2_data.c
@@ -177,6 +177,14 @@ __xfs_dir3_data_check(
while (offset < end) {
struct xfs_dir2_data_unused *dup = bp->b_addr + offset;
struct xfs_dir2_data_entry *dep = bp->b_addr + offset;
+ unsigned int reclen;
+
+ /*
+ * Are the remaining bytes large enough to hold an
+ * unused entry?
+ */
+ if (offset > end - xfs_dir2_data_unusedsize(1))
+ return __this_address;
/*
* If it's unused, look for the space in the bestfree table.
@@ -186,9 +194,13 @@ __xfs_dir3_data_check(
if (be16_to_cpu(dup->freetag) == XFS_DIR2_DATA_FREE_TAG) {
xfs_failaddr_t fa;
+ reclen = xfs_dir2_data_unusedsize(
+ be16_to_cpu(dup->length));
if (lastfree != 0)
return __this_address;
- if (offset + be16_to_cpu(dup->length) > end)
+ if (be16_to_cpu(dup->length) != reclen)
+ return __this_address;
+ if (offset + reclen > end)
return __this_address;
if (be16_to_cpu(*xfs_dir2_data_unused_tag_p(dup)) !=
offset)
@@ -206,10 +218,18 @@ __xfs_dir3_data_check(
be16_to_cpu(bf[2].length))
return __this_address;
}
- offset += be16_to_cpu(dup->length);
+ offset += reclen;
lastfree = 1;
continue;
}
+
+ /*
+ * This is not an unused entry. Are the remaining bytes
+ * large enough for a dirent with a single-byte name?
+ */
+ if (offset > end - xfs_dir2_data_entsize(mp, 1))
+ return __this_address;
+
/*
* It's a real entry. Validate the fields.
* If this is a block directory then make sure it's
@@ -218,9 +238,10 @@ __xfs_dir3_data_check(
*/
if (dep->namelen == 0)
return __this_address;
- if (!xfs_verify_dir_ino(mp, be64_to_cpu(dep->inumber)))
+ reclen = xfs_dir2_data_entsize(mp, dep->namelen);
+ if (offset + reclen > end)
return __this_address;
- if (offset + xfs_dir2_data_entsize(mp, dep->namelen) > end)
+ if (!xfs_verify_dir_ino(mp, be64_to_cpu(dep->inumber)))
return __this_address;
if (be16_to_cpu(*xfs_dir2_data_entry_tag_p(mp, dep)) != offset)
return __this_address;
@@ -244,7 +265,7 @@ __xfs_dir3_data_check(
if (i >= be32_to_cpu(btp->count))
return __this_address;
}
- offset += xfs_dir2_data_entsize(mp, dep->namelen);
+ offset += reclen;
}
/*
* Need to have seen all the entries and all the bestfree slots.
diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
index 7404a9ff1a92..9046d08554e9 100644
--- a/fs/xfs/libxfs/xfs_dir2_priv.h
+++ b/fs/xfs/libxfs/xfs_dir2_priv.h
@@ -187,6 +187,13 @@ void xfs_dir2_sf_put_ftype(struct xfs_mount *mp,
extern int xfs_readdir(struct xfs_trans *tp, struct xfs_inode *dp,
struct dir_context *ctx, size_t bufsize);
+static inline unsigned int
+xfs_dir2_data_unusedsize(
+ unsigned int len)
+{
+ return round_up(len, XFS_DIR2_DATA_ALIGN);
+}
+
static inline unsigned int
xfs_dir2_data_entsize(
struct xfs_mount *mp,
--
2.39.3
next prev parent reply other threads:[~2024-12-18 2:14 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-18 2:13 [PATCH 6.6 CANDIDATE 00/18] xfs backports for 6.6.y (from 6.11) Catherine Hoang
2024-12-18 2:13 ` [PATCH 6.6 CANDIDATE 01/18] xfs: fix the contact address for the sysfs ABI documentation Catherine Hoang
2024-12-18 2:13 ` [PATCH 6.6 CANDIDATE 02/18] xfs: verify buffer, inode, and dquot items every tx commit Catherine Hoang
2024-12-18 2:13 ` [PATCH 6.6 CANDIDATE 03/18] xfs: use consistent uid/gid when grabbing dquots for inodes Catherine Hoang
2024-12-18 2:13 ` [PATCH 6.6 CANDIDATE 04/18] xfs: declare xfs_file.c symbols in xfs_file.h Catherine Hoang
2024-12-18 2:13 ` [PATCH 6.6 CANDIDATE 05/18] xfs: create a new helper to return a file's allocation unit Catherine Hoang
2024-12-18 2:13 ` [PATCH 6.6 CANDIDATE 06/18] xfs: Fix xfs_flush_unmap_range() range for RT Catherine Hoang
2024-12-18 2:14 ` [PATCH 6.6 CANDIDATE 07/18] xfs: Fix xfs_prepare_shift() " Catherine Hoang
2024-12-18 2:14 ` Catherine Hoang [this message]
2024-12-18 2:14 ` [PATCH 6.6 CANDIDATE 09/18] xfs: convert comma to semicolon Catherine Hoang
2024-12-18 2:14 ` [PATCH 6.6 CANDIDATE 10/18] xfs: fix file_path handling in tracepoints Catherine Hoang
2024-12-18 2:14 ` [PATCH 6.6 CANDIDATE 11/18] xfs: remove unused parameter in macro XFS_DQUOT_LOGRES Catherine Hoang
2024-12-18 2:14 ` [PATCH 6.6 CANDIDATE 12/18] xfs: attr forks require attr, not attr2 Catherine Hoang
2024-12-18 2:14 ` [PATCH 6.6 CANDIDATE 13/18] xfs: conditionally allow FS_XFLAG_REALTIME changes if S_DAX is set Catherine Hoang
2024-12-18 2:14 ` [PATCH 6.6 CANDIDATE 14/18] xfs: Fix the owner setting issue for rmap query in xfs fsmap Catherine Hoang
2024-12-18 2:14 ` [PATCH 6.6 CANDIDATE 15/18] xfs: use XFS_BUF_DADDR_NULL for daddrs in getfsmap code Catherine Hoang
2024-12-18 2:14 ` [PATCH 6.6 CANDIDATE 16/18] xfs: Fix missing interval for missing_owner in xfs fsmap Catherine Hoang
2024-12-18 18:32 ` Darrick J. Wong
2024-12-18 2:14 ` [PATCH 6.6 CANDIDATE 17/18] xfs: take m_growlock when running growfsrt Catherine Hoang
2024-12-18 2:14 ` [PATCH 6.6 CANDIDATE 18/18] xfs: reset rootdir extent size hint after growfsrt Catherine Hoang
2024-12-18 18:34 ` [PATCH 6.6 CANDIDATE 00/18] xfs backports for 6.6.y (from 6.11) Darrick J. Wong
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=20241218021411.42144-9-catherine.hoang@oracle.com \
--to=catherine.hoang@oracle.com \
--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