public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Alex Elder <aelder@sgi.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH] xfsprogs: libxfs: Don't forget to initialize the radix tree subsystem
Date: Fri, 14 Oct 2011 09:47:44 +1100	[thread overview]
Message-ID: <20111013224744.GO3159@dastard> (raw)
In-Reply-To: <201110131850.p9DIoEvU006578@stout.americas.sgi.com>

On Thu, Oct 13, 2011 at 01:50:14PM -0500, 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@sgi.com>

/me wonders why valgrind didn't catch that

Anyway, the fix looks good, and well caught!

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com

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

  parent reply	other threads:[~2011-10-13 22:47 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-13 18:50 [PATCH] xfsprogs: libxfs: Don't forget to initialize the radix tree subsystem Alex Elder
2011-10-13 19:51 ` Eric Sandeen
2011-10-13 22:47 ` Dave Chinner [this message]
2011-10-14 12:34 ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111013224744.GO3159@dastard \
    --to=david@fromorbit.com \
    --cc=aelder@sgi.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox