• Darrick J. Wong's avatar
    xfs: allow symlinks with short remote targets · 38de5679
    Darrick J. Wong authored
    An internal user complained about log recovery failing on a symlink
    ("Bad dinode after recovery") with the following (excerpted) format:
    
    core.magic = 0x494e
    core.mode = 0120777
    core.version = 3
    core.format = 2 (extents)
    core.nlinkv2 = 1
    core.nextents = 1
    core.size = 297
    core.nblocks = 1
    core.naextents = 0
    core.forkoff = 0
    core.aformat = 2 (extents)
    u3.bmx[0] = [startoff,startblock,blockcount,extentflag]
    0:[0,12,1,0]
    
    This is a symbolic link with a 297-byte target stored in a disk block,
    which is to say this is a symlink with a remote target.  The forkoff is
    0, which is to say that there's 512 - 176 == 336 bytes in the inode core
    to store the data fork.
    
    Eventually, testing of generic/388 failed with the same inode corruption
    message during inode recovery.  In writing a debugging patch to call
    xfs_dinode_verify on dirty inode log items when we're committing
    transactions, I observed that xfs/298 can reproduce the problem quite
    quickly.
    
    xfs/298 creates a symbolic link, adds some extended attributes, then
    deletes them all.  The test failure occurs when the final removexattr
    also deletes the attr fork because that does not convert the remote
    symlink back into a shortform symlink.  That is how we trip this test.
    The only reason why xfs/298 only triggers with the debug patch added is
    that it deletes the symlink, so the final iflush shows the inode as
    free.
    
    I wrote a quick fstest to emulate the behavior of xfs/298, except that
    it leaves the symlinks on the filesystem after inducing the "corrupt"
    state.  Kernels going back at least as far as 4.18 have written out
    symlink inodes in this manner and prior to 1eb70f54 they did not
    object to reading them back in.
    
    Because we've been writing out inodes this way for quite some time, the
    only way to fix this is to relax the check for symbolic links.
    Directories don't have this problem because di_size is bumped to
    blocksize during the sf->data conversion.
    
    Fixes: 1eb70f54 ("xfs: validate inode fork size against fork format")
    Signed-off-by: default avatar"Darrick J. Wong" <djwong@kernel.org>
    Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
    Signed-off-by: default avatarChandan Babu R <chandanbabu@kernel.org>
    38de5679
xfs_inode_buf.c 22 KB