[PATCH] xfs_db: do bounds checking in frag's scanfunc_bmap

[PATCH] xfs_db: do bounds checking in frag's scanfunc_bmap

[Eric Sandeen]

This is for http://oss.sgi.com/bugzilla/show_bug.cgi?id=842
Bug 842 -  xfs_db crashes on 'frag'

The nrecs in scanfunc_bmap was corrupted & out of bounds, causing
the loop in process_bmbt_reclist to walk well past allocated memory
and eventually segfault.

Add checking to this scanfunc_bmap() similar to that in similar
functions for check, metadump, and repair.

I'm not sure if we can/should print out any more information here...

# db/xfs_db -r -c frag xfs.img 
invalid numrecs (46311) in bmapbtd block
actual 38085, ideal 37731, fragmentation factor 0.93%

Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---

diff --git a/db/frag.c b/db/frag.c
index 925863d..bba91ed 100644
--- a/db/frag.c
+++ b/db/frag.c
@@ -437,15 +437,29 @@ scanfunc_bmap(
 	int			i;
 	xfs_bmbt_ptr_t		*pp;
 	xfs_bmbt_rec_t		*rp;
+	int			nrecs;
+
+	nrecs = be16_to_cpu(block->bb_numrecs);
 
 	if (level == 0) {
+		if (nrecs > mp->m_bmap_dmxr[0]) {
+			dbprintf(_("invalid numrecs (%u) in %s block\n"),
+				   nrecs, typtab[btype].name);
+			return;
+		}
 		rp = XFS_BMBT_REC_ADDR(mp, block, 1);
 		process_bmbt_reclist((xfs_bmbt_rec_32_t *)rp, 
-				be16_to_cpu(block->bb_numrecs), extmapp);
+				nrecs, extmapp);
+		return;
+	}
+
+	if (nrecs > mp->m_bmap_dmxr[1]) {
+		dbprintf(_("invalid numrecs (%u) in %s block\n"),
+			   nrecs, typtab[btype].name);
 		return;
 	}
 	pp = XFS_BMBT_PTR_ADDR(mp, block, 1, mp->m_bmap_dmxr[0]);
-	for (i = 0; i < be16_to_cpu(block->bb_numrecs); i++)
+	for (i = 0; i < nrecs; i++)
 		scan_lbtree(be64_to_cpu(pp[i]), level, scanfunc_bmap, extmapp, 
 									btype);
 }


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_db: do bounds checking in frag's scanfunc_bmap

[Christoph Hellwig]

> @@ -437,15 +437,29 @@ scanfunc_bmap(
>  	int			i;
>  	xfs_bmbt_ptr_t		*pp;
>  	xfs_bmbt_rec_t		*rp;
> +	int			nrecs;
> +
> +	nrecs = be16_to_cpu(block->bb_numrecs);
>  
>  	if (level == 0) {
> +		if (nrecs > mp->m_bmap_dmxr[0]) {
> +			dbprintf(_("invalid numrecs (%u) in %s block\n"),
> +				   nrecs, typtab[btype].name);
> +			return;
> +		}
>  		rp = XFS_BMBT_REC_ADDR(mp, block, 1);
>  		process_bmbt_reclist((xfs_bmbt_rec_32_t *)rp, 
> -				be16_to_cpu(block->bb_numrecs), extmapp);
> +				nrecs, extmapp);
> +		return;
> +	}
> +
> +	if (nrecs > mp->m_bmap_dmxr[1]) {
> +		dbprintf(_("invalid numrecs (%u) in %s block\n"),
> +			   nrecs, typtab[btype].name);
>  		return;
>  	}
>  	pp = XFS_BMBT_PTR_ADDR(mp, block, 1, mp->m_bmap_dmxr[0]);
> -	for (i = 0; i < be16_to_cpu(block->bb_numrecs); i++)
> +	for (i = 0; i < nrecs; i++)
>  		scan_lbtree(be64_to_cpu(pp[i]), level, scanfunc_bmap, extmapp, 
>  									btype);
>  }

Looks good to me.


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

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs