xfs
[Top] [All Lists]

Re: [PATCH] xfsprogs: libxfs: Don't forget to initialize the radix tree

To: Alex Elder <aelder@xxxxxxx>
Subject: Re: [PATCH] xfsprogs: libxfs: Don't forget to initialize the radix tree subsystem
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Thu, 13 Oct 2011 14:51:05 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <201110131850.p9DIoEvU006578@xxxxxxxxxxxxxxxxxxxxxx>
References: <201110131850.p9DIoEvU006578@xxxxxxxxxxxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:7.0.1) Gecko/20110929 Thunderbird/7.0.1
On 10/13/11 1:50 PM, Alex Elder wrote:
> The libxfs code uses radix tree routines to manage a mount
> point's m_perag_tree.  But the radix tree routines assume
> that radix_tree_init() has been called to initialize the
> height_to_maxindex[] global array, and this was not being
> done.
> 
> This showed up when running mkfs.xfs on an ia64 system.  Since
> it wasn't initialized, the array was filled with zeroes.  The
> first time radix_tree_extend() got called (with index 0), the
> height would be set to 1 and all would seem fine.
> 
> The *second* time it got called (with index 1) a problem would
> arise--though we were apparently "lucky" enough for it not to
> matter.  The following loop would simply reference invalid slots
> beyond the end of the array until it happened upon one that was
> non-zero.  (I've expanded the function radix_tree_maxindex() here.)
> 
>         /* Figure out what the height should be.  */
>         height = root->height + 1;
>         while (index > height_to_maxindex[height])
>                 height++;
> 
> As an example, this looped 1937 times before it found a non-zere
> value that would cause it to break out of the loop.
> 
> Even that *seemed* to be OK.  But at the end of mkfs.xfs, when
> it calls libxfs_umount(), non-initialized "slots" are dereferenced
> and we hit a fault.
> 
> Wow.
> 
> Signed-off-by: Alex Elder <aelder@xxxxxxx>

I suppose if we have an init function we may as well call it at
least once.  ;)

Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxx>

> ---
>  libxfs/init.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> Index: b/libxfs/init.c
> ===================================================================
> --- a/libxfs/init.c
> +++ b/libxfs/init.c
> @@ -249,6 +249,8 @@ libxfs_init(libxfs_init_t *a)
>       fd = -1;
>       flags = (a->isreadonly | a->isdirect);
>  
> +     radix_tree_init();
> +
>       if (a->volname) {
>               if(!check_open(a->volname,flags,&rawfile,&blockfile))
>                       goto done;
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
> 

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