The error reported in FS-14-UFS-3 can only happen on UFS/FFS

filesystems that have block pointers that are out-of-range for their
filesystem. These out-of-range block pointers are corrected by
fsck(8) so are only encountered when an unchecked filesystem is
mounted.

A new "untrusted" flag has been added to the generic mount interface
that can be set when mounting media of unknown provenance or integrity.
For example, a daemon that automounts a filesystem on a flash drive
when it is plugged into a system.

This commit adds a test to UFS/FFS that validates all block numbers
before using them. Because checking for out-of-range blocks adds
unnecessary overhead to normal operation, the tests are only done
when the filesystem is mounted as an "untrusted" filesystem.

Reported by:  Christopher Krah, Thomas Barabosch, and Jan-Niclas Hilgert of Fraunhofer FKIE
Reported as:  FS-14-UFS-3: Out of bounds read in write-2 (ffs_alloccg)
Reviewed by:  kib
Sponsored by: Netflix

sys/ufs/ffs/ffs_alloc.c

@@ -1374,7 +1374,7 @@ ffs_blkpref_ufs1(ip, lbn, indx, bap)
 	struct fs *fs;
 	u_int cg, inocg;
 	u_int avgbfree, startcg;
-	ufs2_daddr_t pref;
+	ufs2_daddr_t pref, prevbn;
 
 	KASSERT(indx <= 0 || bap != NULL, ("need non-NULL bap"));
 	mtx_assert(UFS_MTX(ITOUMP(ip)), MA_OWNED);
@@ -1424,7 +1424,15 @@ ffs_blkpref_ufs1(ip, lbn, indx, bap)
 	 * have a block allocated immediately preceding us, then we need
 	 * to decide where to start allocating new blocks.
 	 */
-	if (indx % fs->fs_maxbpg == 0 || bap[indx - 1] == 0) {
+	if (indx ==  0) {
+		prevbn = 0;
+	} else {
+		prevbn = bap[indx - 1];
+		if (UFS_CHECK_BLKNO(ITOVFS(ip), ip->i_number, prevbn,
+		    fs->fs_bsize) != 0)
+			prevbn = 0;
+	}
+	if (indx % fs->fs_maxbpg == 0 || prevbn == 0) {
 		/*
 		 * If we are allocating a directory data block, we want
 		 * to place it in the metadata area.
@@ -1442,10 +1450,10 @@ ffs_blkpref_ufs1(ip, lbn, indx, bap)
 		 * Find a cylinder with greater than average number of
 		 * unused data blocks.
 		 */
-		if (indx == 0 || bap[indx - 1] == 0)
+		if (indx == 0 || prevbn == 0)
 			startcg = inocg + lbn / fs->fs_maxbpg;
 		else
-			startcg = dtog(fs, bap[indx - 1]) + 1;
+			startcg = dtog(fs, prevbn) + 1;
 		startcg %= fs->fs_ncg;
 		avgbfree = fs->fs_cstotal.cs_nbfree / fs->fs_ncg;
 		for (cg = startcg; cg < fs->fs_ncg; cg++)
@@ -1463,7 +1471,7 @@ ffs_blkpref_ufs1(ip, lbn, indx, bap)
 	/*
 	 * Otherwise, we just always try to lay things out contiguously.
 	 */
-	return (bap[indx - 1] + fs->fs_frag);
+	return (prevbn + fs->fs_frag);
 }
 
 /*
@@ -1479,7 +1487,7 @@ ffs_blkpref_ufs2(ip, lbn, indx, bap)
 	struct fs *fs;
 	u_int cg, inocg;
 	u_int avgbfree, startcg;
-	ufs2_daddr_t pref;
+	ufs2_daddr_t pref, prevbn;
 
 	KASSERT(indx <= 0 || bap != NULL, ("need non-NULL bap"));
 	mtx_assert(UFS_MTX(ITOUMP(ip)), MA_OWNED);
@@ -1529,7 +1537,15 @@ ffs_blkpref_ufs2(ip, lbn, indx, bap)
 	 * have a block allocated immediately preceding us, then we need
 	 * to decide where to start allocating new blocks.
 	 */
-	if (indx % fs->fs_maxbpg == 0 || bap[indx - 1] == 0) {
+	if (indx ==  0) {
+		prevbn = 0;
+	} else {
+		prevbn = bap[indx - 1];
+		if (UFS_CHECK_BLKNO(ITOVFS(ip), ip->i_number, prevbn,
+		    fs->fs_bsize) != 0)
+			prevbn = 0;
+	}
+	if (indx % fs->fs_maxbpg == 0 || prevbn == 0) {
 		/*
 		 * If we are allocating a directory data block, we want
 		 * to place it in the metadata area.
@@ -1547,10 +1563,10 @@ ffs_blkpref_ufs2(ip, lbn, indx, bap)
 		 * Find a cylinder with greater than average number of
 		 * unused data blocks.
 		 */
-		if (indx == 0 || bap[indx - 1] == 0)
+		if (indx == 0 || prevbn == 0)
 			startcg = inocg + lbn / fs->fs_maxbpg;
 		else
-			startcg = dtog(fs, bap[indx - 1]) + 1;
+			startcg = dtog(fs, prevbn) + 1;
 		startcg %= fs->fs_ncg;
 		avgbfree = fs->fs_cstotal.cs_nbfree / fs->fs_ncg;
 		for (cg = startcg; cg < fs->fs_ncg; cg++)
@@ -1568,7 +1584,7 @@ ffs_blkpref_ufs2(ip, lbn, indx, bap)
 	/*
 	 * Otherwise, we just always try to lay things out contiguously.
 	 */
-	return (bap[indx - 1] + fs->fs_frag);
+	return (prevbn + fs->fs_frag);
 }
 
 /*

sys/ufs/ffs/ffs_balloc.c

@@ -99,6 +99,7 @@ ffs_balloc_ufs1(struct vnode *vp, off_t startoffset, int size,
 	struct fs *fs;
 	ufs1_daddr_t nb;
 	struct buf *bp, *nbp;
+	struct mount *mp;
 	struct ufsmount *ump;
 	struct indir indirs[UFS_NIADDR + 2];
 	int deallocated, osize, nsize, num, i, error;
@@ -113,6 +114,7 @@ ffs_balloc_ufs1(struct vnode *vp, off_t startoffset, int size,
 	ip = VTOI(vp);
 	dp = ip->i_din1;
 	fs = ITOFS(ip);
+	mp = ITOVFS(ip);
 	ump = ITOUMP(ip);
 	lbn = lblkno(fs, startoffset);
 	size = blkoff(fs, startoffset) + size;
@@ -295,6 +297,11 @@ ffs_balloc_ufs1(struct vnode *vp, off_t startoffset, int size,
 		}
 		bap = (ufs1_daddr_t *)bp->b_data;
 		nb = bap[indirs[i].in_off];
+		if ((error = UFS_CHECK_BLKNO(mp, ip->i_number, nb,
+		    fs->fs_bsize)) != 0) {
+			brelse(bp);
+			goto fail;
+		}
 		if (i == num)
 			break;
 		i += 1;
@@ -580,6 +587,7 @@ ffs_balloc_ufs2(struct vnode *vp, off_t startoffset, int size,
 	ufs_lbn_t lbn, lastlbn;
 	struct fs *fs;
 	struct buf *bp, *nbp;
+	struct mount *mp;
 	struct ufsmount *ump;
 	struct indir indirs[UFS_NIADDR + 2];
 	ufs2_daddr_t nb, newb, *bap, pref;
@@ -593,6 +601,7 @@ ffs_balloc_ufs2(struct vnode *vp, off_t startoffset, int size,
 	ip = VTOI(vp);
 	dp = ip->i_din2;
 	fs = ITOFS(ip);
+	mp = ITOVFS(ip);
 	ump = ITOUMP(ip);
 	lbn = lblkno(fs, startoffset);
 	size = blkoff(fs, startoffset) + size;
@@ -888,6 +897,11 @@ ffs_balloc_ufs2(struct vnode *vp, off_t startoffset, int size,
 		}
 		bap = (ufs2_daddr_t *)bp->b_data;
 		nb = bap[indirs[i].in_off];
+		if ((error = UFS_CHECK_BLKNO(mp, ip->i_number, nb,
+		    fs->fs_bsize)) != 0) {
+			brelse(bp);
+			goto fail;
+		}
 		if (i == num)
 			break;
 		i += 1;

sys/ufs/ffs/ffs_extern.h

@@ -69,6 +69,7 @@ ufs2_daddr_t ffs_blkpref_ufs2(struct inode *, ufs_lbn_t, int, ufs2_daddr_t *);
 void	ffs_blkrelease_finish(struct ufsmount *, u_long);
 u_long	ffs_blkrelease_start(struct ufsmount *, struct vnode *, ino_t);
 uint32_t ffs_calc_sbhash(struct fs *);
+int	ffs_check_blkno(struct mount *, ino_t, ufs2_daddr_t, int);
 int	ffs_checkfreefile(struct fs *, struct vnode *, ino_t);
 void	ffs_clrblock(struct fs *, u_char *, ufs1_daddr_t);
 void	ffs_clusteracct(struct fs *, struct cg *, ufs1_daddr_t, int);

sys/ufs/ffs/ffs_softdep.c

@@ -8124,6 +8124,7 @@ indir_trunc(freework, dbn, lbn)
 	struct buf *bp;
 	struct fs *fs;
 	struct indirdep *indirdep;
+	struct mount *mp;
 	struct ufsmount *ump;
 	ufs1_daddr_t *bap1;
 	ufs2_daddr_t nb, nnb, *bap2;
@@ -8133,7 +8134,8 @@ indir_trunc(freework, dbn, lbn)
 	int goingaway, freedeps, needj, level, cnt, i;
 
 	freeblks = freework->fw_freeblks;
-	ump = VFSTOUFS(freeblks->fb_list.wk_mp);
+	mp = freeblks->fb_list.wk_mp;
+	ump = VFSTOUFS(mp);
 	fs = ump->um_fs;
 	/*
 	 * Get buffer of block pointers to be freed.  There are three cases:
@@ -8226,6 +8228,9 @@ indir_trunc(freework, dbn, lbn)
 	 */
 	key = ffs_blkrelease_start(ump, freeblks->fb_devvp, freeblks->fb_inum);
 	for (i = freework->fw_off; i < NINDIR(fs); i++, nb = nnb) {
+		if (UFS_CHECK_BLKNO(mp, freeblks->fb_inum, nb,
+		    fs->fs_bsize) != 0)
+			nb = 0;
 		if (i != NINDIR(fs) - 1) {
 			if (ufs1fmt)
 				nnb = bap1[i+1];

sys/ufs/ffs/ffs_subr.c

@@ -154,6 +154,55 @@ ffs_load_inode(struct buf *bp, struct inode *ip, struct fs *fs, ino_t ino)
 	ip->i_gid = dip2->di_gid;
 	return (0);
 }
+
+/*
+ * Verify that a filesystem block number is a valid data block.
+ * This routine is only called on untrusted filesystems.
+ */
+int
+ffs_check_blkno(struct mount *mp, ino_t inum, ufs2_daddr_t daddr, int blksize)
+{
+	struct fs *fs;
+	struct ufsmount *ump;
+	ufs2_daddr_t end_daddr;
+	int cg, havemtx;
+
+	KASSERT((mp->mnt_flag & MNT_UNTRUSTED) != 0,
+	    ("ffs_check_blkno called on a trusted file system"));
+	ump = VFSTOUFS(mp);
+	fs = ump->um_fs;
+	cg = dtog(fs, daddr);
+	end_daddr = daddr + numfrags(fs, blksize);
+	/*
+	 * Verify that the block number is a valid data block. Also check
+	 * that it does not point to an inode block or a superblock. Accept
+	 * blocks that are unalloacted (0) or part of snapshot metadata
+	 * (BLK_NOCOPY or BLK_SNAP).
+	 *
+	 * Thus, the block must be in a valid range for the filesystem and
+	 * either in the space before a backup superblock (except the first
+	 * cylinder group where that space is used by the bootstrap code) or
+	 * after the inode blocks and before the end of the cylinder group.
+	 */
+	if ((uint64_t)daddr <= BLK_SNAP ||
+	    ((uint64_t)end_daddr <= fs->fs_size &&
+	    ((cg > 0 && end_daddr <= cgsblock(fs, cg)) ||
+	    (daddr >= cgdmin(fs, cg) &&
+	    end_daddr <= cgbase(fs, cg) + fs->fs_fpg))))
+		return (0);
+	if ((havemtx = mtx_owned(UFS_MTX(ump))) == 0)
+		UFS_LOCK(ump);
+	if (ppsratecheck(&ump->um_last_integritymsg,
+	    &ump->um_secs_integritymsg, 1)) {
+		UFS_UNLOCK(ump);
+		uprintf("\n%s: inode %jd, out-of-range indirect block "
+		    "number %jd\n", mp->mnt_stat.f_mntonname, inum, daddr);
+		if (havemtx)
+			UFS_LOCK(ump);
+	} else if (!havemtx)
+		UFS_UNLOCK(ump);
+	return (EINTEGRITY);
+}
 #endif /* _KERNEL */
 
 /*

sys/ufs/ffs/ffs_vfsops.c

@@ -919,6 +919,10 @@ ffs_mountfs(devvp, mp, td)
 	ump->um_ifree = ffs_ifree;
 	ump->um_rdonly = ffs_rdonly;
 	ump->um_snapgone = ffs_snapgone;
+	if ((mp->mnt_flag & MNT_UNTRUSTED) != 0)
+		ump->um_check_blkno = ffs_check_blkno;
+	else
+		ump->um_check_blkno = NULL;
 	mtx_init(UFS_MTX(ump), "FFS", "FFS Lock", MTX_DEF);
 	ffs_oldfscompat_read(fs, ump, fs->fs_sblockloc);
 	fs->fs_ronly = ronly;

sys/ufs/ufs/ufs_bmap.c

@@ -267,8 +267,16 @@ ufs_bmaparray(vp, bn, bnp, nbp, runp, runb)
 		if (error != 0)
 			return (error);
 
-		if (I_IS_UFS1(ip)) {
+		if (I_IS_UFS1(ip))
 			daddr = ((ufs1_daddr_t *)bp->b_data)[ap->in_off];
+		else
+			daddr = ((ufs2_daddr_t *)bp->b_data)[ap->in_off];
+		if ((error = UFS_CHECK_BLKNO(mp, ip->i_number, daddr,
+		     mp->mnt_stat.f_iosize)) != 0) {
+			bqrelse(bp);
+			return (error);
+		}
+		if (I_IS_UFS1(ip)) {
 			if (num == 1 && daddr && runp) {
 				for (bn = ap->in_off + 1;
 				    bn < MNINDIR(ump) && *runp < maxrun &&
@@ -287,7 +295,6 @@ ufs_bmaparray(vp, bn, bnp, nbp, runp, runb)
 			}
 			continue;
 		}
-		daddr = ((ufs2_daddr_t *)bp->b_data)[ap->in_off];
 		if (num == 1 && daddr && runp) {
 			for (bn = ap->in_off + 1;
 			    bn < MNINDIR(ump) && *runp < maxrun &&

sys/ufs/ufs/ufsmount.h

@@ -102,6 +102,8 @@ struct ufsmount {
 	u_int	um_flags;			/* (i) filesystem flags */
 	struct	timeval um_last_fullmsg;	/* (i) last full msg time */
 	int	um_secs_fullmsg;		/* (i) seconds since full msg */
+	struct	timeval um_last_integritymsg;	/* (i) last integrity msg */
+	int	um_secs_integritymsg;		/* (i) secs since integ msg */
 	u_int	um_trim_inflight;		/* (i) outstanding trim count */
 	u_int	um_trim_inflight_blks;		/* (i) outstanding trim blks */
 	u_long	um_trim_total;			/* (i) total trim count */
@@ -121,6 +123,7 @@ struct ufsmount {
 	void	(*um_ifree)(struct ufsmount *, struct inode *);
 	int	(*um_rdonly)(struct inode *);
 	void	(*um_snapgone)(struct inode *);
+	int	(*um_check_blkno)(struct mount *, ino_t, daddr_t, int);
 };
 
 /*
@@ -145,6 +148,9 @@ struct ufsmount {
 #define	UFS_IFREE(aa, bb) ((aa)->um_ifree(aa, bb))
 #define	UFS_RDONLY(aa) (ITOUMP(aa)->um_rdonly(aa))
 #define	UFS_SNAPGONE(aa) (ITOUMP(aa)->um_snapgone(aa))
+#define	UFS_CHECK_BLKNO(aa, bb, cc, dd) 		\
+	(VFSTOUFS(aa)->um_check_blkno == NULL ? 0 :	\
+	 VFSTOUFS(aa)->um_check_blkno(aa, bb, cc, dd))
 
 #define	UFS_LOCK(aa)	mtx_lock(&(aa)->um_lock)
 #define	UFS_UNLOCK(aa)	mtx_unlock(&(aa)->um_lock)