vxfs_readdir() and vxfs_find_entry() walk a directory page with an intra-page offset, pg_ofs, and dereference kaddr + pg_ofs without bounding it. Nothing validates the page first: vxfs_check_page() was never implemented, only a commented out call survives in vxfs_get_page(). Three on-disk values steer the walk unchecked. VXFS_DIRBLKOV(), 2 * d_nhash + 4, is added at a dirblock boundary and the entry is dereferenced right after; the enclosing while (pg_ofs < PAGE_SIZE && pos < limit) only gates the next iteration. d_nhash can move the dereference 32 pages from the start of the page. d_reclen is added at the end of each iteration. There the loop condition does run, but it only proves that pg_ofs is inside the page, not that an entry header is: pg_ofs of PAGE_SIZE - 2 passes it, and d_reclen at offset 4 of the next entry is off the page. d_namelen is never compared to its record. vxfs_readdir() passes it to dir_emit() as the length of d_name, which starts at offset 10 of the entry. filldir64() rejects PATH_MAX and above, but only after verify_dirent_name() has run memchr() over the name, and shorter names are copied out - up to 4095 bytes past the page. vxfs_find_entry() memcmp()s up to 256. A crafted image reaches all three, vxfs_readdir() from getdents64(2) and vxfs_find_entry() from a path walk. KASAN reports a 2-byte read past the page in vxfs_readdir(), a read in memchr() below filldir64(), and a read in memcmp() below vxfs_lookup(). Bound them: leave the page if the overhead moved pg_ofs out of it, require the entry header and then the whole record to fit in the page, and skip an entry whose name does not fit in its record. A record shorter than its own header is refused too, so a free entry cannot slide the next one by less than an entry. Where the page is abandoned, advance pos to its end so the outer loop keeps making progress. vxfs_readdir() also ended the walk on a page that emitted nothing, its per-page rc conflating a full buffer with an empty page, and the new checks add another way to reach that; track the full buffer directly. Valid images are unaffected: a record holds its own name and does not cross its dirblock, dirblocks do not cross a block, and sb_set_blocksize() has already refused a block size larger than a page. Found by XBOW. Fixes: 12495ea3ac47 ("freevxfs: refactor readdir and lookup code") Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Reported-by: Federico Kirschbaum Cc: stable@vger.kernel.org Signed-off-by: Baul Lee --- v2, in reply to the Sashiko review Christian pointed at. Its three points are right. v1 checked pg_ofs only after the dirblock overhead, which does not cover the dereference: pg_ofs of PAGE_SIZE - 2 passes that check too, and d_reclen alone reaches it. A kernel carrying v1 still reports both reads. The d_namelen one is the worse of the two and predates the 2016 refactor, hence the second Fixes tag. The premature end of the walk is real but is not memory safety; it is in here because the new checks add a way to reach it, and I can split it out if you prefer. Not taken: 4 byte alignment for d_reclen. It cannot be established from d_reclen alone - VXFS_DIRBLKOV() is 2 * d_nhash + 4, so an odd d_nhash already starts a dirblock's entries two bytes off, and such an image mounts and lists fine before and after this patch. A short record is refused, which is the half that needs no guess. The rest belongs with an implementation of vxfs_check_page(). Verified on 7.2.0-rc5 with KASAN, on crafted images built from the driver's own headers: before the patch getdents64() and stat() produce the three reports above, after it none, and well-formed images list unchanged. Review: https://sashiko.dev/#/patchset/20260726062514.43756-1-baul.lee%40xbow.com v1: https://lore.kernel.org/linux-fsdevel/20260726062514.43756-1-baul.lee@xbow.com/ fs/freevxfs/vxfs_lookup.c | 69 +++++++++++++++++++++++++++++++-------- 1 file changed, 56 insertions(+), 13 deletions(-) diff --git a/fs/freevxfs/vxfs_lookup.c b/fs/freevxfs/vxfs_lookup.c index 138e08de976e..50c428c63126 100644 --- a/fs/freevxfs/vxfs_lookup.c +++ b/fs/freevxfs/vxfs_lookup.c @@ -78,6 +78,7 @@ vxfs_find_entry(struct inode *ip, struct dentry *dp, struct page **ppp) while (pg_ofs < PAGE_SIZE && pos < limit) { struct vxfs_direct *de; + int reclen, nlen; if ((pos & (bsize - 1)) < 4) { struct vxfs_dirblk *dbp = @@ -87,6 +88,14 @@ vxfs_find_entry(struct inode *ip, struct dentry *dp, struct page **ppp) pos += overhead; pg_ofs += overhead; + if (pg_ofs >= PAGE_SIZE) + break; + } + + /* the entry header must fit in the page */ + if (pg_ofs + VXFS_NAMEMIN > PAGE_SIZE) { + pos += PAGE_SIZE - pg_ofs; + break; } de = (struct vxfs_direct *)(kaddr + pg_ofs); @@ -96,12 +105,25 @@ vxfs_find_entry(struct inode *ip, struct dentry *dp, struct page **ppp) break; } - pg_ofs += fs16_to_cpu(sbi, de->d_reclen); - pos += fs16_to_cpu(sbi, de->d_reclen); + /* and so must the whole record, header included */ + reclen = fs16_to_cpu(sbi, de->d_reclen); + if (reclen < VXFS_NAMEMIN || + pg_ofs + reclen > PAGE_SIZE) { + pos += PAGE_SIZE - pg_ofs; + break; + } + + pg_ofs += reclen; + pos += reclen; if (!de->d_ino) continue; - if (namelen != fs16_to_cpu(sbi, de->d_namelen)) + /* the name must fit in the record */ + nlen = fs16_to_cpu(sbi, de->d_namelen); + if (VXFS_NAMEMIN + nlen > reclen) + continue; + + if (namelen != nlen) continue; if (!memcmp(name, de->d_name, namelen)) { *ppp = pp; @@ -218,7 +240,7 @@ vxfs_readdir(struct file *fp, struct dir_context *ctx) struct page *pp; char *kaddr; int pg_ofs = pos & ~PAGE_MASK; - int rc = 0; + bool full = false; pp = vxfs_get_page(ip->i_mapping, pos >> PAGE_SHIFT); if (IS_ERR(pp)) @@ -228,6 +250,7 @@ vxfs_readdir(struct file *fp, struct dir_context *ctx) while (pg_ofs < PAGE_SIZE && pos < limit) { struct vxfs_direct *de; + int reclen, nlen; if ((pos & (bsize - 1)) < 4) { struct vxfs_dirblk *dbp = @@ -237,6 +260,14 @@ vxfs_readdir(struct file *fp, struct dir_context *ctx) pos += overhead; pg_ofs += overhead; + if (pg_ofs >= PAGE_SIZE) + break; + } + + /* the entry header must fit in the page */ + if (pg_ofs + VXFS_NAMEMIN > PAGE_SIZE) { + pos += PAGE_SIZE - pg_ofs; + break; } de = (struct vxfs_direct *)(kaddr + pg_ofs); @@ -246,23 +277,35 @@ vxfs_readdir(struct file *fp, struct dir_context *ctx) break; } - pg_ofs += fs16_to_cpu(sbi, de->d_reclen); - pos += fs16_to_cpu(sbi, de->d_reclen); + /* and so must the whole record, header included */ + reclen = fs16_to_cpu(sbi, de->d_reclen); + if (reclen < VXFS_NAMEMIN || + pg_ofs + reclen > PAGE_SIZE) { + pos += PAGE_SIZE - pg_ofs; + break; + } + + pg_ofs += reclen; + pos += reclen; if (!de->d_ino) continue; - rc = dir_emit(ctx, de->d_name, - fs16_to_cpu(sbi, de->d_namelen), - fs32_to_cpu(sbi, de->d_ino), - DT_UNKNOWN); - if (!rc) { + /* the name must fit in the record */ + nlen = fs16_to_cpu(sbi, de->d_namelen); + if (VXFS_NAMEMIN + nlen > reclen) + continue; + + if (!dir_emit(ctx, de->d_name, nlen, + fs32_to_cpu(sbi, de->d_ino), + DT_UNKNOWN)) { /* the dir entry was not read, fix pos. */ - pos -= fs16_to_cpu(sbi, de->d_reclen); + pos -= reclen; + full = true; break; } } vxfs_put_page(pp); - if (!rc) + if (full) break; } -- 2.53.0