xfs
[Top] [All Lists]

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

To: Eric Sandeen <sandeen@xxxxxxxxxxx>
Subject: Re: [PATCH V2] xfs: add a few more verifier tests
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Tue, 9 Sep 2014 11:47:20 +1000
Cc: Eric Sandeen <sandeen@xxxxxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <53F3A726.8080305@xxxxxxxxxxx>
References: <53F2C103.8030607@xxxxxxxxxx> <53F3A726.8080305@xxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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@xxxxxxxxxx>
> --

Looks good.

Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>

-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>
  • Re: [PATCH V2] xfs: add a few more verifier tests, Dave Chinner <=