Re: [PATCH V2] xfs: add a few more verifier tests

[Dave Chinner <david@fromorbit.com>]

On Tue, Aug 19, 2014 at 02:36:06PM -0500, Eric Sandeen wrote:
> These were exposed by fsfuzzer runs; without them we fail
> in various exciting and sometimes convoluted ways when we
> encounter disk corruption.
> 
> Without the MAXLEVELS tests we tend to walk off the end of
> an array in a loop like this:
> 
>         for (i = 0; i < cur->bc_nlevels; i++) {
>                 if (cur->bc_bufs[i])
> 
> Without the dirblklog test we try to allocate more memory
> than we could possibly hope for and loop forever:
> 
> xfs_dabuf_map()
> 	nfsb = mp->m_dir_geo->fsbcount;
> 	irecs = kmem_zalloc(sizeof(irec) * nfsb, KM_SLEEP...
> 
> As for the logbsize check, that's the convoluted one.
> 
> If logbsize is specified at mount time, it's sanitized
> in xfs_parseargs; in particular it makes sure that it's
> not > XLOG_MAX_RECORD_BSIZE.
>     
> If not specified at mount time, it comes from the superblock
> via sb_logsunit; this is limited to 256k at mkfs time as well;
> it's copied into m_logbsize in xfs_finish_flags().
>     
> However, if for some reason the on-disk value is corrupt and
> too large, nothing catches it.  It's a circuitous path, but
> that size eventually finds its way to places that make the kernel
> very unhappy, leading to oopses in xlog_pack_data() because we
> use the size as an index into iclog->ic_data, but the array
> is not necessarily that big.
> 
> Anyway - bounds checking when we read from disk is a good thing!
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> --

Looks good.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

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