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

Andrew Morton <[email protected]>
Newsgroups dev.linux.lists.ocfs2-devel,org.kernel.vger.linux-kernel,org.kernel.vger.stable
Message-ID <[email protected]>
On Thu,  6 Aug 2026 20:21:33 +0800 Zhan Xusheng <[email protected]> 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 <[email protected]>
> Fixes: ccd979bdbce9 ("[PATCH] OCFS2: The Second Oracle Cluster Filesystem")
> Cc: [email protected]

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/[email protected]

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

	https://sashiko.dev/#/patchset/[email protected]

Anyway, I'll queue this one and shall await maintainer input.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.