public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: xfs@oss.sgi.com
Subject: [PATCH v2 0/3] xfs: avoid overflow of attr3 leaf block headers with 64k blocks
Date: Mon, 30 Mar 2015 11:12:42 -0400	[thread overview]
Message-ID: <1427728365-15842-1-git-send-email-bfoster@redhat.com> (raw)

Hi all,

Here's v2 of the series previously posted here:

	http://oss.sgi.com/archives/xfs/2015-02/msg00516.html

This version incorporates the feedback from v1 to use separate helpers
for firstused conversion. I've also added a 3rd patch to kill what looks
like a (now unnecessary) overflow check.

To give a little more context... it appears that this overflow might
have been handled previously in the attr code by reducing the firstused
value by the required offset alignment and leaking the last few bytes of
the block. In fact, xfs_attr3_leaf_create() used to have a similar check
that was removed in:

	517c22207b04 xfs: add CRCs to attr leaf blocks

... so I suspect that introduced the regression. Both of those
!firstused checks appear to exist as of the original git import.
Unfortunately, I haven't seen them documented anywhere, but I can't
think of any other purpose for them.

Finally, I took a quick look at userspace... First, this depends on
xfs_da_geometry, which is not yet available there. So I suspect this
will depend on a larger xfsprogs update. IIRC, something of that nature
is pending or in progress..?

Second, from what I can see, nothing functional actually needs to
change. xfs_repair already does the correct thing by using a larger type
to compute firstused. It's written directly to the on-disk header and
will overflow at 64k, but that naturally results in the correct value of
0 on-disk. We should definitely make the code explicit, but this is
resolved simply by updating the conversion functions in libxfs.

Thoughts, reviews, flames appreciated...

Brian

v2:
- Created separate firstused conversion helpers to isolate the overflow
  management to a single point.
- Added patch 3/3 to kill an unnecessary overflow check.
- Comment cleanups, additions.

Brian Foster (3):
  xfs: pass attr geometry to attr leaf header conversion functions
  xfs: use larger in-core attr firstused field and detect overflow
  xfs: kill unnecessary firstused overflow check on attr3 leaf removal

 fs/xfs/libxfs/xfs_attr_leaf.c | 150 ++++++++++++++++++++++++++++++++----------
 fs/xfs/libxfs/xfs_attr_leaf.h |   6 +-
 fs/xfs/libxfs/xfs_da_format.h |  14 +++-
 fs/xfs/xfs_attr_inactive.c    |   3 +-
 fs/xfs/xfs_attr_list.c        |   9 ++-
 5 files changed, 139 insertions(+), 43 deletions(-)

-- 
1.9.3

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

             reply	other threads:[~2015-03-30 15:12 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-30 15:12 Brian Foster [this message]
2015-03-30 15:12 ` [PATCH v2 1/3] xfs: pass attr geometry to attr leaf header conversion functions Brian Foster
2015-03-30 15:12 ` [PATCH v2 2/3] xfs: use larger in-core attr firstused field and detect overflow Brian Foster
2015-03-30 15:12 ` [PATCH v2 3/3] xfs: kill unnecessary firstused overflow check on attr3 leaf removal Brian Foster

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=1427728365-15842-1-git-send-email-bfoster@redhat.com \
    --to=bfoster@redhat.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