On 9/30/18 1:05 AM, Dave Chinner wrote:
> On Sun, Sep 30, 2018 at 12:06:44AM -0500, Eric Sandeen wrote:
>> On 9/29/18 10:25 PM, Dave Chinner wrote:
>>> On Mon, Sep 24, 2018 at 10:00:39PM -0500, Eric Sandeen wrote:
...
>>> So, yeah, I think that this check needs to be different because I
>>> think we could have symlinks as short at 56 bytes in extent format,
>>> even when the inode has no attribute fork...
>>
>> Hrmph. And yet, xfs_repair:
>>
>> static int
>> process_symlink_extlist(xfs_mount_t *mp, xfs_ino_t lino, xfs_dinode_t *dino)
>> {
>> xfs_fileoff_t expected_offset;
>> xfs_bmbt_rec_t *rp;
>> xfs_bmbt_irec_t irec;
>> int numrecs;
>> int i;
>> int max_blocks;
>>
>> if (be64_to_cpu(dino->di_size) <= XFS_DFORK_DSIZE(dino, mp)) {
>> if (dino->di_format == XFS_DINODE_FMT_LOCAL)
>> return 0;
>> do_warn(
>> _("mismatch between format (%d) and size (%" PRId64 ") in symlink ino %" PRIu64 "\n"),
>> dino->di_format,
>> (int64_t)be64_to_cpu(dino->di_size), lino);
>> return 1;
>> }
>>
>> seems to clearly call "non-local symlink with size < XFS_DFORK_DSIZE" corruption.
>> What gives?
>
> Seems to me like another cases that the verifiers have uncovered
> another situation that even repair doesn't handle correctly. (Which
> is why I like to look at these things from first principles, rather
> than just from the "what does reapir do" POV?). It's like to be rare
> because who removes all the attributes on a file apart from when
> unlinking the inode?
Sure, I get it that repair is not the canonical truth for format, I was
just surprised that you hit it fairly quickly with the kernel verifier,
but apparently didn't see it prior to this patch in a post-test repair run.
In fact I don't think I've ever seen this check fail in repair. So it
just seemed odd. It might be interesting to see if we can provoke it in
xfs_repair?
> xfs_attr_fork_remove() has set the di_forkoff back to zero
> when the last attribute is removed from the inode for a long time
> (I stopped looking when I got to ~2008...), so this isn't a new
> situation. i.e. trying to define what are valid values has
> demonstrated that there are more cases we need to take into account
> than anyone has realised.
Yeah, I have a vague recollection of other bugs related to the fork
offset that led to the sense of nearly nondeterministic behavior for
when things move in and out of local.
-Eric
