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@xxxxxxxxx> 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@xxxxxxxxxxxxxxxxx>
> Fixes: ccd979bdbce9 ("[PATCH] OCFS2: The Second Oracle Cluster Filesystem")
> Cc: stable@xxxxxxxxxxxxxxx

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@xxxxxxxxxx

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

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

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