[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@xxxxxxxxxxxx>
Signed-off-by: Farhad Alemi <farhad.alemi@xxxxxxxxxxxx>
---
 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