xfs
[Top] [All Lists]

Re: [PATCH V2] xfsdump: prevent segfault in cb_add_inogrp

To: Rich Johnston <rjohnston@xxxxxxx>
Subject: Re: [PATCH V2] xfsdump: prevent segfault in cb_add_inogrp
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 27 Aug 2015 08:21:15 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <55DE3651.5030803@xxxxxxx>
References: <20150924193241.975348815@xxxxxxxxxxxxxxxxxxxxxxx> <55EDF3BA.60409@xxxxxxx> <20150826213746.GC3902@dastard> <55DE3651.5030803@xxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Wed, Aug 26, 2015 at 04:57:37PM -0500, Rich Johnston wrote:
> Dave,
> 
> On 08/26/2015 04:37 PM, Dave Chinner wrote:
> >On Wed, Aug 26, 2015 at 12:36:42PM -0500, rjohnston@xxxxxxx wrote:
> >>The call to memset will segfault because the offset for the first
> >>parameter is done twice. We are using pointer math to do the
> >>calculation.
> >>The first time is when calculating oldsize, the size of i2gseg_t
> >>is accounted for.
> >>    oldsize = (numsegs - SEGPERHNK) * sizeof(i2gseg_t);
> >>Then in the call to memset, oldsize is again multiplied by the size
> >>of i2gmap_t.
> >>    memset(inomap.i2gmap + oldsize, ...)
> >>    
> >>i2gmap holds the used inodes in each chunk. When there are 2^31 chunk
> >>entries, it could describe 2^31 (1 inode/chunk)- 2^40 (64 inodes/chunk).
> >>
> >>With 100s of millions of inodes there are enough entries to wrap the
> >>32 bit variable oldsize.
> >>
> >>Switching to use array index notation instead of calculating the
> >>pointer address twice ;) would resolve this issue. The unneeded
> >>local variable oldsize can be removed as well.
> >>
> Per your other comment I will add a bounds check after calculating numsegs:
>       if (numsegs < 0)
>               return -1;
> 
> The description above will change to:
> 
> Adding a bounds check (numsegs < 0) and switching to use array
> index notation instead of calculating the pointer address twice ;)
> would resolve this issue. The unneeded local variable oldsize
> can be removed as well.

Sounds good. Thanks, Rich.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>