* Re: [PATCH] freevxfs: fix off-page read on a crafted dirblock
2026-07-26 6:25 [PATCH] freevxfs: fix off-page read on a crafted dirblock Baul Lee
@ 2026-07-28 7:24 ` Christian Brauner
2026-08-04 7:00 ` [PATCH v2] freevxfs: fix off-page reads in the directory walkers Baul Lee
1 sibling, 0 replies; 3+ messages in thread
From: Christian Brauner @ 2026-07-28 7:24 UTC (permalink / raw)
To: Baul Lee; +Cc: linux-fsdevel, linux-kernel, hch, federico.kirschbaum, stable
On 2026-07-26 15:25 +0900, Baul Lee wrote:
> vxfs_readdir() and vxfs_find_entry() both advance the intra-page offset
> pg_ofs by VXFS_DIRBLKOV(sbi, dbp) at each dirblock boundary and then
> dereference
>
> de = (struct vxfs_direct *)(kaddr + pg_ofs);
>
> without re-checking pg_ofs against PAGE_SIZE in between. The enclosing
> while (pg_ofs < PAGE_SIZE && pos < limit) test only gates loop
> re-entry, not the deref that follows the addition in the same
> iteration.
>
> VXFS_DIRBLKOV() is 2 * d_nhash + 4, and d_nhash is an unvalidated
> on-disk __fs16, so the overhead is attacker arithmetic of up to roughly
> 32 pages. kaddr is page_address() of the single order-0 page-cache
> page returned by vxfs_get_page(), so nothing else bounds the addition
> and the de->d_reclen load lands off the mapped page. Directory pages
> are consumed unvalidated: vxfs_check_page() was never implemented, only
> a commented-out call survives.
>
> Both sites are reachable on any mounted crafted freevxfs image, the
> vxfs_readdir() one from an unprivileged getdents64(2) and the
> vxfs_find_entry() one from any path walk into the directory. When
> d_ino is non-zero vxfs_readdir() also feeds the off-page bytes to
> dir_emit(), so they reach userspace. KASAN reports a 2-byte
> out-of-bounds read in vxfs_readdir().
>
> Re-check pg_ofs against PAGE_SIZE right after the addition and before
> the deref in both loops, restoring the invariant they already assume.
> The added break leaves the page the same way the existing zero-d_reclen
> break does. Well-formed directories, whose overhead never pushes
> pg_ofs past a single dirblock, are unaffected.
>
> Discovered by XBOW, triaged by Baul Lee <baul.lee@xbow.com>
>
> Fixes: 12495ea3ac47 ("freevxfs: refactor readdir and lookup code")
> Reported-by: Federico Kirschbaum <federico.kirschbaum@xbow.com>
> Reported-by: Baul Lee <baul.lee@xbow.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Baul Lee <baul.lee@xbow.com>
> ---
> fs/freevxfs/vxfs_lookup.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/fs/freevxfs/vxfs_lookup.c b/fs/freevxfs/vxfs_lookup.c
> index 138e08de976e..b068f93359fb 100644
> --- a/fs/freevxfs/vxfs_lookup.c
> +++ b/fs/freevxfs/vxfs_lookup.c
> @@ -87,6 +87,8 @@ vxfs_find_entry(struct inode *ip, struct dentry *dp, struct page **ppp)
>
> pos += overhead;
> pg_ofs += overhead;
> + if (pg_ofs >= PAGE_SIZE)
> + break;
> }
> de = (struct vxfs_direct *)(kaddr + pg_ofs);
>
> @@ -237,6 +239,8 @@ vxfs_readdir(struct file *fp, struct dir_context *ctx)
>
> pos += overhead;
> pg_ofs += overhead;
> + if (pg_ofs >= PAGE_SIZE)
> + break;
There's valid Sashiko reviews that you want to read through.
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH v2] freevxfs: fix off-page reads in the directory walkers
2026-07-26 6:25 [PATCH] freevxfs: fix off-page read on a crafted dirblock Baul Lee
2026-07-28 7:24 ` Christian Brauner
@ 2026-08-04 7:00 ` Baul Lee
1 sibling, 0 replies; 3+ messages in thread
From: Baul Lee @ 2026-08-04 7:00 UTC (permalink / raw)
To: linux-fsdevel, linux-kernel
Cc: hch, federico.kirschbaum, Baul Lee, stable, brauner
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 <federico.kirschbaum@xbow.com>
Cc: stable@vger.kernel.org
Signed-off-by: Baul Lee <baul.lee@xbow.com>
---
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
^ permalink raw reply [flat|nested] 3+ messages in thread