[PATCH] ocfs2: bound-check dir entries in the readdir re-validation scan

[PATCH] ocfs2: bound-check dir entries in the readdir re-validation scan

[Zhan Xusheng]

When the inode version changed since the last readdir(),
ocfs2_dir_foreach_blk_el() re-scans the directory block from its start to
relocate the current position:

	for (i = 0; i < sb->s_blocksize && i < offset; ) {
		de = (struct ocfs2_dir_entry *)(bh->b_data + i);
		if (le16_to_cpu(de->rec_len) < OCFS2_DIR_REC_LEN(1))
			break;
		i += le16_to_cpu(de->rec_len);
	}

The loop dereferences de->rec_len (at byte offset 8 within the entry)
guarded only by i < sb->s_blocksize.  `offset` is derived from ctx->pos,
which userspace controls via lseek() on the directory fd, so i can reach
the last bytes of the block; reading de->rec_len then reads a few bytes
past the s_blocksize-sized block buffer (an out-of-bounds read).

The main emit loop below already guards this via ocfs2_check_dir_entry(),
which rejects entries too close to the buffer end before touching de.
Apply the same lower bound to the re-validation scan so that a full
minimal directory entry is known to fit before de is dereferenced.  For a
consistent directory this changes nothing: entries are at least
OCFS2_DIR_REC_LEN(1) bytes, so no valid entry starts in the excluded tail.

Found by the sashiko review tool; fix approach suggested by Joseph Qi.

Suggested-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Fixes: ccd979bdbce9 ("[PATCH] OCFS2: The Second Oracle Cluster Filesystem")
Cc: stable@vger.kernel.org
Link: https://sashiko.dev/#/patchset/20260806022044.167962-1-zhanxusheng@xiaomi.com
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 fs/ocfs2/dir.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
index d7fc3cccf2f4..3b606b4c04dd 100644
--- a/fs/ocfs2/dir.c
+++ b/fs/ocfs2/dir.c
@@ -1903,7 +1903,8 @@ static int ocfs2_dir_foreach_blk_el(struct inode *inode,
 		 * dirent right now.  Scan from the start of the block
 		 * to make sure. */
 		if (!inode_eq_iversion(inode, *f_version)) {
-			for (i = 0; i < sb->s_blocksize && i < offset; ) {
+			for (i = 0; i + OCFS2_DIR_REC_LEN(1) <= sb->s_blocksize &&
+			     i < offset;) {
 				de = (struct ocfs2_dir_entry *) (bh->b_data + i);
 				/* It's too expensive to do a full
 				 * dirent test each time round this
-- 
2.43.0

Re: [PATCH] ocfs2: bound-check dir entries in the readdir re-validation scan

[Andrew Morton]

On Thu,  6 Aug 2026 20:21:33 +0800 Zhan Xusheng <zhanxusheng1024@gmail.com> wrote:

> When the inode version changed since the last readdir(),
> ocfs2_dir_foreach_blk_el() re-scans the directory block from its start to
> relocate the current position:
> 
> 	for (i = 0; i < sb->s_blocksize && i < offset; ) {
> 		de = (struct ocfs2_dir_entry *)(bh->b_data + i);
> 		if (le16_to_cpu(de->rec_len) < OCFS2_DIR_REC_LEN(1))
> 			break;
> 		i += le16_to_cpu(de->rec_len);
> 	}
> 
> The loop dereferences de->rec_len (at byte offset 8 within the entry)
> guarded only by i < sb->s_blocksize.  `offset` is derived from ctx->pos,
> which userspace controls via lseek() on the directory fd, so i can reach
> the last bytes of the block; reading de->rec_len then reads a few bytes
> past the s_blocksize-sized block buffer (an out-of-bounds read).
> 
> The main emit loop below already guards this via ocfs2_check_dir_entry(),
> which rejects entries too close to the buffer end before touching de.
> Apply the same lower bound to the re-validation scan so that a full
> minimal directory entry is known to fit before de is dereferenced.  For a
> consistent directory this changes nothing: entries are at least
> OCFS2_DIR_REC_LEN(1) bytes, so no valid entry starts in the excluded tail.
> 
> Found by the sashiko review tool; fix approach suggested by Joseph Qi.

Thanks.

When fixing a bug, please always describe the userspace-visible runtime
effects of that bug.

> Suggested-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> Fixes: ccd979bdbce9 ("[PATCH] OCFS2: The Second Oracle Cluster Filesystem")
> Cc: stable@vger.kernel.org

Especially when proposing a backport.

I asked Gemini "what are the userspace-visible effects of this bug"
then pasted in your email.  The answer was, basically, "there aren't any".

	https://share.gemini.google/FiBJNo4qIz7J

So I don't believe that a cc:stable is justified,
Documentation/process/stable-kernel-rules.rst says "it must fix a real
bug that bothers people".

So if maintainers are agreeable I think I'll remove that cc:stable. 
But I think the -stable maintainers will go and backport it anyway
because of the Fixes: (thereby breaking their own rules ;)).  We can
stop that happening by removing the Fixes: also.

> Link: https://sashiko.dev/#/patchset/20260806022044.167962-1-zhanxusheng@xiaomi.com

Your patch prompted Sashiko to complain about more pre-existing things:

	https://sashiko.dev/#/patchset/20260806122133.956847-1-zhanxusheng@xiaomi.com

Anyway, I'll queue this one and shall await maintainer input.

Re: [PATCH] ocfs2: bound-check dir entries in the readdir re-validation scan

[Joseph Qi]

On 8/6/26 8:21 PM, Zhan Xusheng wrote:
> When the inode version changed since the last readdir(),
> ocfs2_dir_foreach_blk_el() re-scans the directory block from its start to
> relocate the current position:
> 
> 	for (i = 0; i < sb->s_blocksize && i < offset; ) {
> 		de = (struct ocfs2_dir_entry *)(bh->b_data + i);
> 		if (le16_to_cpu(de->rec_len) < OCFS2_DIR_REC_LEN(1))
> 			break;
> 		i += le16_to_cpu(de->rec_len);
> 	}
> 
> The loop dereferences de->rec_len (at byte offset 8 within the entry)
> guarded only by i < sb->s_blocksize.  `offset` is derived from ctx->pos,
> which userspace controls via lseek() on the directory fd, so i can reach
> the last bytes of the block; reading de->rec_len then reads a few bytes
> past the s_blocksize-sized block buffer (an out-of-bounds read).
> 
> The main emit loop below already guards this via ocfs2_check_dir_entry(),
> which rejects entries too close to the buffer end before touching de.
> Apply the same lower bound to the re-validation scan so that a full
> minimal directory entry is known to fit before de is dereferenced.  For a
> consistent directory this changes nothing: entries are at least
> OCFS2_DIR_REC_LEN(1) bytes, so no valid entry starts in the excluded tail.
> 
> Found by the sashiko review tool; fix approach suggested by Joseph Qi.
> 
> Suggested-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> Fixes: ccd979bdbce9 ("[PATCH] OCFS2: The Second Oracle Cluster Filesystem")
> Cc: stable@vger.kernel.org
> Link: https://sashiko.dev/#/patchset/20260806022044.167962-1-zhanxusheng@xiaomi.com
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> ---
>  fs/ocfs2/dir.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> index d7fc3cccf2f4..3b606b4c04dd 100644
> --- a/fs/ocfs2/dir.c
> +++ b/fs/ocfs2/dir.c
> @@ -1903,7 +1903,8 @@ static int ocfs2_dir_foreach_blk_el(struct inode *inode,
>  		 * dirent right now.  Scan from the start of the block
>  		 * to make sure. */
>  		if (!inode_eq_iversion(inode, *f_version)) {
> -			for (i = 0; i < sb->s_blocksize && i < offset; ) {
> +			for (i = 0; i + OCFS2_DIR_REC_LEN(1) <= sb->s_blocksize &&
> +			     i < offset;) {
>  				de = (struct ocfs2_dir_entry *) (bh->b_data + i);
>  				/* It's too expensive to do a full
>  				 * dirent test each time round this

It seems we have to guard two things according to sashiko review comments.
1st is to safely read this record's length, and 2nd is make sure the record's
length stay inside the block.

The above targets the 1st, so the 2nd is still missing. e.g.

if (le16_to_cpu(de->rec_len) < OCFS2_DIR_REC_LEN(1)) ||
    i + le16_to_cpu(de->rec_len) > sb->s_blocksize)
	break;

Thanks,
Joseph