xfs
[Top] [All Lists]

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

To: rjohnston@xxxxxxx, xfs@xxxxxxxxxxx
Subject: Re: [PATCH 1/2] xfsdump: use 64bit local variables in inode.c
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Fri, 21 Aug 2015 12:03:16 -0500
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20150821193241.899709228@xxxxxxxxxxxxxxxxxxxxxxx>
References: <20150821193047.661578219@xxxxxxxxxxxxxxxxxxxxxxx> <20150821193241.899709228@xxxxxxxxxxxxxxxxxxxxxxx>
On 8/21/15 9:01 AM, rjohnston@xxxxxxx 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@xxxxxxx>
> ---
>  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@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
> 

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