Re: [PATCH 1/2] xfsdump: use 64bit local variables in inode.c

[Eric Sandeen <sandeen@sandeen.net>]

On 8/21/15 9:01 AM, rjohnston@sgi.com wrote:
> The memset in cb_add_inogrp will segfault when the index oldsize
> overflows. In cb_add_inogrp(), the temp variables used in
> calculating the new i2gmap segment offset should be int64 instead
> of intgen_t (int32).
> 
> Fix this:
>   a. simplify the calculation of oldsize by moving it
>      before hnkmaplen is incremented.

I think it'd make more sense to put a) in the second patch; it's
unrelated to the variable size changes.

Still wrapping my head around what these variables are for, but
makes sense at first glance.

Thanks,
-Eric

>   b. change the index variables int64 to prevent overflow. 
> 
> Signed-off-by: Rich Johnston <rjohnston@sgi.com>
> ---
>  dump/inomap.c |   41 +++++++++++++++++++++--------------------
>  1 file changed, 21 insertions(+), 20 deletions(-)
> 
> Index: b/dump/inomap.c
> ===================================================================
> --- a/dump/inomap.c
> +++ b/dump/inomap.c
> @@ -71,7 +71,7 @@ static intgen_t cb_context( bool_t last,
>  			    drange_t *,
>  			    startpt_t *,
>  			    size_t,
> -			    intgen_t,
> +			    int64_t,
>  			    bool_t,
>  			    bool_t *);
>  static void cb_context_free( void );
> @@ -96,7 +96,7 @@ static off64_t estimate_dump_space( xfs_
>  
>  /* inomap primitives
>   */
> -static intgen_t inomap_init( intgen_t igrpcnt );
> +static intgen_t inomap_init( int64_t igrpcnt );
>  static void inomap_add( void *, xfs_ino_t ino, gen_t gen, intgen_t );
>  static intgen_t inomap_set_state( void *, xfs_ino_t ino, intgen_t );
>  static void inomap_set_gen(void *, xfs_ino_t, gen_t );
> @@ -160,7 +160,7 @@ inomap_build( jdm_fshandle_t *fshandlep,
>  	xfs_bstat_t *bstatbufp;
>  	size_t bstatbuflen;
>  	bool_t pruneneeded = BOOL_FALSE;
> -	intgen_t igrpcnt = 0;
> +	int64_t igrpcnt = 0;
>  	intgen_t stat;
>  	intgen_t rval;
>  
> @@ -449,7 +449,7 @@ cb_context( bool_t last,
>  	    drange_t *resumerangep,
>  	    startpt_t *startptp,
>  	    size_t startptcnt,
> -	    intgen_t igrpcnt,
> +	    int64_t igrpcnt,
>  	    bool_t skip_unchanged_dirs,
>  	    bool_t *pruneneededp )
>  {
> @@ -949,14 +949,14 @@ struct i2gseg {
>  typedef struct i2gseg i2gseg_t;
>  
>  typedef struct seg_addr {
> -	intgen_t hnkoff;
> -	intgen_t segoff;
> -	intgen_t inooff;
> +	int64_t hnkoff;
> +	int64_t segoff;
> +	int64_t inooff;
>  } seg_addr_t;
>  
>  static struct inomap {
>  	hnk_t *hnkmap;
> -	intgen_t hnkmaplen;
> +	int64_t hnkmaplen;
>  	i2gseg_t *i2gmap;
>  	seg_addr_t lastseg;
>  } inomap;
> @@ -1040,7 +1040,7 @@ SEG_GET_BITS( seg_t *segp, xfs_ino_t ino
>  /* context for inomap construction - initialized by map_init
>   */
>  static intgen_t
> -inomap_init( intgen_t igrpcnt )
> +inomap_init( int64_t igrpcnt )
>  {
>  	ASSERT( sizeof( hnk_t ) == HNKSZ );
>  
> @@ -1066,7 +1066,7 @@ inomap_getsz( void )
>  static inline bool_t
>  inomap_validaddr( seg_addr_t *addrp )
>  {
> -	intgen_t maxseg;
> +	int64_t maxseg;
>  
>  	if ( addrp->hnkoff < 0 || addrp->hnkoff > inomap.lastseg.hnkoff )
>  		return BOOL_FALSE;
> @@ -1093,13 +1093,13 @@ inomap_addr2seg( seg_addr_t *addrp )
>  	return &hunkp->seg[addrp->segoff];
>  }
>  
> -static inline intgen_t
> +static inline int64_t
>  inomap_addr2segix( seg_addr_t *addrp )
>  {
>  	return ( addrp->hnkoff * SEGPERHNK ) + addrp->segoff;
>  }
>  
> -static inline intgen_t
> +static inline int64_t
>  inomap_lastseg( intgen_t hnkoff )
>  {
>  	if ( hnkoff == inomap.lastseg.hnkoff )
> @@ -1125,8 +1125,11 @@ cb_add_inogrp( void *arg1, intgen_t fsfd
>  		lastsegp->segoff = 0;
>  
>  		if (lastsegp->hnkoff == inomap.hnkmaplen) {
> -			intgen_t numsegs;
> -			intgen_t oldsize;
> +			int64_t numsegs;
> +			int64_t oldsize;
> +
> +			oldsize = inomap.hnkmaplen * SEGPERHNK
> +				  * sizeof(i2gseg_t);
>  
>  			inomap.hnkmaplen++;
>  			inomap.hnkmap = (hnk_t *)
> @@ -1140,8 +1143,6 @@ cb_add_inogrp( void *arg1, intgen_t fsfd
>  				return -1;
>  
>  			/* zero the new portion of the i2gmap */
> -			oldsize = (numsegs - SEGPERHNK) * sizeof(i2gseg_t);
> -
>  			memset(inomap.i2gmap + oldsize,
>  			       0,
>  			       SEGPERHNK * sizeof(i2gseg_t));
> @@ -1199,8 +1200,8 @@ static bool_t
>  inomap_find_hnk( seg_addr_t *addrp, xfs_ino_t ino )
>  {
>  	hnk_t *hunkp;
> -	intgen_t lower;
> -	intgen_t upper;
> +	int64_t lower;
> +	int64_t upper;
>  
>  	lower = 0;
>  	upper = inomap.lastseg.hnkoff;
> @@ -1231,8 +1232,8 @@ static bool_t
>  inomap_find_seg( seg_addr_t *addrp, xfs_ino_t ino )
>  {
>  	seg_t *segp;
> -	intgen_t lower;
> -	intgen_t upper;
> +	int64_t lower;
> +	int64_t upper;
>  
>  	if ( !inomap_validaddr( addrp ) ) {
>  		inomap_reset_context( addrp );
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
> 

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