On Fri, Jul 08, 2016 at 09:14:55AM +1000, Dave Chinner wrote:

On Thu, Jul 07, 2016 at 12:18:13PM -0700, Darrick J. Wong wrote:

quoted

On Thu, Jul 07, 2016 at 02:41:56PM -0400, Brian Foster wrote:

quoted

quoted

+	if (!xfs_sb_version_hasrmapbt(&mp->m_sb))
+		return false;
+	if (!xfs_btree_sblock_v5hdr_verify(bp))
+		return false;
+
+	level = be16_to_cpu(block->bb_level);
+	if (pag && pag->pagf_init) {
+		if (level >= pag->pagf_levels[XFS_BTNUM_RMAPi])
+			return false;
+	} else if (level >= mp->m_rmap_maxlevels)
+		return false;

It looks like the above (level >= mp->m_rmap_maxlevels) check could be
independent (rather than an 'else). Otherwise looks good:

Hmmm.... at first I wondered, "Shouldn't we have already checked that
pag->pagf_levels[XFS_BTNUM_RMAPi] <= mp->m_rmap_maxlevels?"  But then I
realized that no, we don't do that anywhere.  Nor does the bnobt/cntbt
verifier.  Am I missing something?

(Yes, I am.)

It should have been ranged checked when the AGF is first read in
(i.e. in the verifier), in ASSERTS every time xfs_alloc_read_agf()
is called after initialisation, and then every time the verifier is
run on write of the AGF.

You're right.  I missed that. :(

quoted

I did see that we at least check the AGF/AGI levels to make sure they don't
overflow XFS_BTREE_MAXLEVELS, so we're probably fine here.

Precisely - if the AGF verifier doesn't have a max level check in it
for the rmapbt, then we need to add one there.

There's a check there, so we're fine.

--D

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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