From: Alex Elder <aelder@sgi.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Ben Hutchings <ben@decadent.org.uk>,
Carlos Maiolino <cmaiolino@redhat.com>,
xfs@oss.sgi.com
Subject: Re: [PATCH] Fix possible memory corruption in xfs_readlink
Date: Wed, 2 Nov 2011 15:22:35 -0500 [thread overview]
Message-ID: <1320265355.3145.53.camel@doink> (raw)
In-Reply-To: <20111102194507.GA14429@infradead.org>
On Wed, 2011-11-02 at 15:45 -0400, Christoph Hellwig wrote:
> We should validate that the value isn't negative in xfs_iformat_*,
> although we currently don't do that. It already verified that it
> fits into the XFS_DFORK_DSIZE, which should take care of fitting
> into 32-bits. Adding another explicit check probably won't hurt,
> given that XFS_DFORK_DSIZE is calculated dynamically based on the
> fork offset.
>
That's true, but there are other places where it gets
updated, yet not defensively validated. For example,
in xfs_dir2_shrink_inode(), if:
fsbno > (INT64_MAX >> mp->m_sb.sb_blocklog)
then the (signed) di_size field would be assigned
a value that exceeded its max representable value,
producing unreliable (implementation-defined) results.
That may well be an impossible situation, but it's
not obvious without really looking at the code.
It's a bit of a can of worms, which is why I suggested
just testing for this (unlikely) condition in
xfs_readlink() for now.
-Alex
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2011-11-02 20:22 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-11-01 14:14 [PATCH] Fix possible memory corruption in xfs_readlink Ben Hutchings
2011-11-02 17:52 ` Alex Elder
2011-11-02 19:45 ` Christoph Hellwig
2011-11-02 20:22 ` Alex Elder [this message]
2011-11-07 16:10 ` [PATCH, updated] xfs: " Alex Elder
2011-11-07 16:31 ` Carlos Maiolino
2011-11-08 14:38 ` Christoph Hellwig
-- strict thread matches above, loose matches on Subject: below --
2011-10-18 4:18 [PATCH] " Carlos Maiolino
2011-10-18 6:52 ` Christoph Hellwig
2011-10-18 13:59 ` Alex Elder
2011-10-18 14:25 ` Eric Sandeen
2011-10-17 21:05 Carlos Maiolino
2011-10-17 22:39 ` Alex Elder
2011-10-17 22:43 ` Dave Chinner
2011-10-18 1:28 ` Carlos Maiolino
2011-10-17 15:30 Carlos Maiolino
2011-10-17 14:00 ` Christoph Hellwig
2011-10-17 17:24 ` Eric Sandeen
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=1320265355.3145.53.camel@doink \
--to=aelder@sgi.com \
--cc=ben@decadent.org.uk \
--cc=cmaiolino@redhat.com \
--cc=hch@infradead.org \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.