From: Eric Biggers <ebiggers3@gmail.com>
To: Andreas Dilger <adilger@dilger.ca>
Cc: linux-ext4@vger.kernel.org, Theodore Ts'o <tytso@mit.edu>,
Eric Biggers <ebiggers@google.com>
Subject: Re: [PATCH] libext2fs: correctly subtract xattr blocks on bigalloc filesystems
Date: Wed, 24 May 2017 20:10:38 -0700 [thread overview]
Message-ID: <20170525031038.GA630@zzz> (raw)
In-Reply-To: <91D3FA03-A23E-45C4-AA3D-575C52D59408@dilger.ca>
Hi Andreas,
On Tue, May 23, 2017 at 10:19:35AM -0600, Andreas Dilger wrote:
> On May 21, 2017, at 12:23 AM, Eric Biggers <ebiggers3@gmail.com> wrote:
> >
> > From: Eric Biggers <ebiggers@google.com>
> >
> > ext2fs_inode_data_blocks2() calculates an inode's data block count by
> > subtracting the external xattr block, if any, from the total blocks.
> > But on bigalloc filesystems, the xattr "block" is actually a whole
> > cluster, so ext2fs_inode_data_blocks2() would return a too-large value.
> >
> > It seems this could have caused several different problems, but the one
> > I encountered was that xfstest generic/399 failed in the "bigalloc"
> > config because e2fsck incorrectly considered a symlink on the filesystem
> > to be corrupted at the end of the test. This happened because e2fsck
> > incorrectly calculated a nonzero data block count for a "fast" symlink
> > with an external xattr block and therefore treated it as a "slow"
> > symlink, which failed validation.
>
> I thought we changed this to detect "fast" inodes by i_size < 60 rather
> than using the blocks count, because the blocks count was (and apparently
> continues to be) unreliable for determining fast vs. slow symlinks.
>
> However, ext4_inode_is_fast_symlink() still checks blocks count. In
> "[PATCH] ext4: fix reading new encrypted symlinks on no-journal filesystems"
> we discussed whether this was safe, and it appears to be OK from my
> analysis.
>
> We just continue to hit problems when extrapolating various blocks counts
> to detect fast symlinks rather than just using the same mechanism we use
> at creation time, which is "len > EXT4_N_BLOCKS * 4".
>
> Cheers, Andreas
>
Yes, I still think we probably should do that. This bug needed to be fixed
anyway though, since ext2fs_inode_data_blocks2() is used for a bit more than
just distinguishing between fast and slow symlinks.
Eric
next prev parent reply other threads:[~2017-05-25 3:10 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-21 6:23 [PATCH] libext2fs: correctly subtract xattr blocks on bigalloc filesystems Eric Biggers
2017-05-23 16:19 ` Andreas Dilger
2017-05-25 3:10 ` Eric Biggers [this message]
2017-05-25 1:54 ` Theodore Ts'o
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=20170525031038.GA630@zzz \
--to=ebiggers3@gmail.com \
--cc=adilger@dilger.ca \
--cc=ebiggers@google.com \
--cc=linux-ext4@vger.kernel.org \
--cc=tytso@mit.edu \
/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.