[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@xxxxxxxxxxxx>
Suggested-by: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Link: https://lore.kernel.org/lkml/ahVAXetxKFqukggG@xxxxxxxxxxxxx/
Signed-off-by: Farhad Alemi <farhad.alemi@xxxxxxxxxxxx>
---
Changes since v1
(https://lore.kernel.org/lkml/CA+0ovCji0_+S3pJBd+-00oXHe5FXd7ATr8BWANN+MoUQYSs8=w@xxxxxxxxxxxxxx/):
  * 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@xxxxxxxxxxxxx/).
  * 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