[PATCH] freevxfs: don't BUG() on duplicate OLT entries

[PATCH] freevxfs: don't BUG() on duplicate OLT entries

[Farhad Alemi]

vxfs_read_olt() walks the object location table (OLT) and dispatches
each entry to vxfs_get_fshead() or vxfs_get_ilist() based on its type.
Both helpers BUG_ON() if the per-superblock field they write
(vsi_fshino / vsi_iext) is already non-zero, on the unstated assumption
that the OLT contains at most one entry of each type. The on-disk
format does not enforce that assumption, so a crafted image with two or
more FSHEAD or ILIST entries trips the BUG at mount time:

  kernel BUG at fs/freevxfs/vxfs_olt.c:28!
  RIP: vxfs_get_ilist fs/freevxfs/vxfs_olt.c:28 [inline]
       vxfs_read_olt+0x665/0x680 fs/freevxfs/vxfs_olt.c:92
  Call Trace:
   vxfs_fill_super+0x4cd/0x830 fs/freevxfs/vxfs_super.c:251
   get_tree_bdev_flags+0x436/0x500 fs/super.c:1698
   vfs_get_tree+0x97/0x2b0 fs/super.c:1758
   do_new_mount+0x32e/0xa50 fs/namespace.c:3728
   __se_sys_mount+0x322/0x420 fs/namespace.c:4216

Rather than treat a malformed on-disk OLT as an internal kernel
invariant violation, treat it as bad input and reject the image. Make
vxfs_get_fshead() and vxfs_get_ilist() return int (0 on success,
-EINVAL on the duplicate condition that previously BUG'd) and check
the return in vxfs_read_olt() so its existing fail label runs:
brelse(bp) and return -EINVAL. The malformed image is rejected at
mount(2) rather than crashing the kernel or being silently accepted
with attacker-chosen vsi_fshino / vsi_iext values.

The existing post-loop sanity check

	return (infp->vsi_fshino && infp->vsi_iext) ? 0 : -EINVAL;

continues to require both fields to be non-zero, so images that supply
no FSHEAD or no ILIST are still rejected as before. Behavior for
well-formed images is unchanged.

The third BUG_ON() in this file (vxfs_oblock's check that bsize divides
sbp->s_blocksize) is a different bug class with a different reach path
and is left for a separate change.

Reported-by: Farhad Alemi <farhad.alemi@berkeley.edu>
Signed-off-by: Farhad Alemi <farhad.alemi@berkeley.edu>
---
 fs/freevxfs/vxfs_olt.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/fs/freevxfs/vxfs_olt.c b/fs/freevxfs/vxfs_olt.c
index 23f35187c289..21c66af16c1a 100644
--- a/fs/freevxfs/vxfs_olt.c
+++ b/fs/freevxfs/vxfs_olt.c
@@ -15,18 +15,22 @@
 #include "vxfs_extern.h"


-static inline void
+static inline int
 vxfs_get_fshead(struct vxfs_oltfshead *fshp, struct vxfs_sb_info *infp)
 {
-	BUG_ON(infp->vsi_fshino);
+	if (infp->vsi_fshino)
+		return -EINVAL;
 	infp->vsi_fshino = fs32_to_cpu(infp, fshp->olt_fsino[0]);
+	return 0;
 }

-static inline void
+static inline int
 vxfs_get_ilist(struct vxfs_oltilist *ilistp, struct vxfs_sb_info *infp)
 {
-	BUG_ON(infp->vsi_iext);
+	if (infp->vsi_iext)
+		return -EINVAL;
 	infp->vsi_iext = fs32_to_cpu(infp, ilistp->olt_iext[0]);
+	return 0;
 }

 static inline u_long
@@ -86,10 +90,12 @@ vxfs_read_olt(struct super_block *sbp, u_long bsize)
 		
 		switch (fs32_to_cpu(infp, ocp->olt_type)) {
 		case VXFS_OLT_FSHEAD:
-			vxfs_get_fshead((struct vxfs_oltfshead *)oaddr, infp);
+			if (vxfs_get_fshead((struct vxfs_oltfshead *)oaddr, infp))
+				goto fail;
 			break;
 		case VXFS_OLT_ILIST:
-			vxfs_get_ilist((struct vxfs_oltilist *)oaddr, infp);
+			if (vxfs_get_ilist((struct vxfs_oltilist *)oaddr, infp))
+				goto fail;
 			break;
 		}

-- 
2.43.0

Re: [PATCH] freevxfs: don't BUG() on duplicate OLT entries

[Christoph Hellwig]

On Mon, May 25, 2026 at 06:10:59PM -0700, Farhad Alemi wrote:
> +static inline int
>  vxfs_get_fshead(struct vxfs_oltfshead *fshp, struct vxfs_sb_info *infp)
>  {
> -	BUG_ON(infp->vsi_fshino);
> +	if (infp->vsi_fshino)
> +		return -EINVAL;
>  	infp->vsi_fshino = fs32_to_cpu(infp, fshp->olt_fsino[0]);
> +	return 0;
>  }
> 
> -static inline void
> +static inline int
>  vxfs_get_ilist(struct vxfs_oltilist *ilistp, struct vxfs_sb_info *infp)
>  {
> -	BUG_ON(infp->vsi_iext);
> +	if (infp->vsi_iext)
> +		return -EINVAL;

I'd still keep a WARN_ON_ONCE in the conditional here as this is an
unexpected condition.

> +			if (vxfs_get_fshead((struct vxfs_oltfshead *)oaddr, infp))

Overly long line here.  Otherwise looks good.

For the next resend please also include Christian Brauner
<christian@brauner.io> as he picks up the freevxfs patches through the
VFS tree.

Re: [PATCH] freevxfs: don't BUG() on duplicate OLT entries

[Christoph Hellwig]

On Tue, May 26, 2026 at 11:49:47AM -0700, Farhad Alemi wrote:
> Hi Christoph,
> 
> Thanks for the review. Before sending v2, I tested WARN_ON_ONCE against the
> original reproducer, and on a panic_on_warn=y kernel it still panics (the
> WARN fires and panic_on_warn promotes it).
> Would pr_warn_once() work instead, to keep the diagnostic without the
> panic_on_warn coupling?

That's fine.  People who set panic_on_warn=y for anyting but debug
kernels deserve this.

> Happy to respin either way — just thought I'd check before doing so.

Sure, it's always good to ask!

[PATCH v2] freevxfs: don't BUG() on duplicate OLT entries

[Farhad Alemi]

vxfs_read_olt() walks the object location table (OLT) and dispatches
each entry to vxfs_get_fshead() or vxfs_get_ilist() based on its type.
Both helpers BUG_ON() if the per-superblock field they write
(vsi_fshino / vsi_iext) is already non-zero, on the unstated assumption
that the OLT contains at most one entry of each type. The on-disk
format does not enforce that assumption, so a crafted image with two or
more FSHEAD or ILIST entries trips the BUG at mount time:

  kernel BUG at fs/freevxfs/vxfs_olt.c:28!
  RIP: vxfs_get_ilist fs/freevxfs/vxfs_olt.c:28 [inline]
       vxfs_read_olt+0x665/0x680 fs/freevxfs/vxfs_olt.c:92
  Call Trace:
   vxfs_fill_super+0x4cd/0x830 fs/freevxfs/vxfs_super.c:251
   get_tree_bdev_flags+0x436/0x500 fs/super.c:1698
   vfs_get_tree+0x97/0x2b0 fs/super.c:1758
   do_new_mount+0x32e/0xa50 fs/namespace.c:3728
   __se_sys_mount+0x322/0x420 fs/namespace.c:4216

Rather than treat a malformed on-disk OLT as an internal kernel
invariant violation, treat it as bad input and reject the image. Make
vxfs_get_fshead() and vxfs_get_ilist() return int (0 on success,
-EINVAL on the duplicate condition that previously BUG'd) and check
the return in vxfs_read_olt() so its existing fail label runs:
brelse(bp) and return -EINVAL. The condition is still flagged with
WARN_ON_ONCE() to log the unexpected state; the WARN is retained at
the maintainer's request. The malformed image is rejected at mount(2)
rather than being silently accepted with attacker-chosen vsi_fshino /
vsi_iext values.

The existing post-loop sanity check

	return (infp->vsi_fshino && infp->vsi_iext) ? 0 : -EINVAL;

continues to require both fields to be non-zero, so images that supply
no FSHEAD or no ILIST are still rejected as before. Behavior for
well-formed images is unchanged.

The third BUG_ON() in this file (vxfs_oblock's check that bsize divides
sbp->s_blocksize) is a different bug class with a different reach path
and is left for a separate change.

Reported-by: Farhad Alemi <farhad.alemi@berkeley.edu>
Suggested-by: Christoph Hellwig <hch@infradead.org>
Link: https://lore.kernel.org/lkml/ahVAXetxKFqukggG@infradead.org/
Signed-off-by: Farhad Alemi <farhad.alemi@berkeley.edu>
---
Changes since v1
(https://lore.kernel.org/lkml/CA+0ovCji0_+S3pJBd+-00oXHe5FXd7ATr8BWANN+MoUQYSs8=w@mail.gmail.com/):
  * Wrap the duplicate-OLT-entry checks with WARN_ON_ONCE() so the
    unexpected condition is still logged, per Christoph Hellwig
    (https://lore.kernel.org/lkml/ahVAXetxKFqukggG@infradead.org/).
  * Break the if-call lines in vxfs_read_olt() under 80 cols.
---
 fs/freevxfs/vxfs_olt.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/fs/freevxfs/vxfs_olt.c b/fs/freevxfs/vxfs_olt.c
index 23f35187c289..1c0694324135 100644
--- a/fs/freevxfs/vxfs_olt.c
+++ b/fs/freevxfs/vxfs_olt.c
@@ -15,18 +15,22 @@
 #include "vxfs_extern.h"


-static inline void
+static inline int
 vxfs_get_fshead(struct vxfs_oltfshead *fshp, struct vxfs_sb_info *infp)
 {
-	BUG_ON(infp->vsi_fshino);
+	if (WARN_ON_ONCE(infp->vsi_fshino))
+		return -EINVAL;
 	infp->vsi_fshino = fs32_to_cpu(infp, fshp->olt_fsino[0]);
+	return 0;
 }

-static inline void
+static inline int
 vxfs_get_ilist(struct vxfs_oltilist *ilistp, struct vxfs_sb_info *infp)
 {
-	BUG_ON(infp->vsi_iext);
+	if (WARN_ON_ONCE(infp->vsi_iext))
+		return -EINVAL;
 	infp->vsi_iext = fs32_to_cpu(infp, ilistp->olt_iext[0]);
+	return 0;
 }

 static inline u_long
@@ -86,10 +90,14 @@ vxfs_read_olt(struct super_block *sbp, u_long bsize)
 		
 		switch (fs32_to_cpu(infp, ocp->olt_type)) {
 		case VXFS_OLT_FSHEAD:
-			vxfs_get_fshead((struct vxfs_oltfshead *)oaddr, infp);
+			if (vxfs_get_fshead((struct vxfs_oltfshead *)oaddr,
+					    infp))
+				goto fail;
 			break;
 		case VXFS_OLT_ILIST:
-			vxfs_get_ilist((struct vxfs_oltilist *)oaddr, infp);
+			if (vxfs_get_ilist((struct vxfs_oltilist *)oaddr,
+					   infp))
+				goto fail;
 			break;
 		}

-- 
2.43.0

Re: [PATCH v2] freevxfs: don't BUG() on duplicate OLT entries

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Christian, can you queue this up through the VFS tree?