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.