OSDN Git Service

xfs: fix inode fork extent count overflow
authorDave Chinner <dchinner@redhat.com>
Thu, 17 Oct 2019 20:40:33 +0000 (13:40 -0700)
committerDarrick J. Wong <darrick.wong@oracle.com>
Mon, 21 Oct 2019 16:04:58 +0000 (09:04 -0700)
[commit message is verbose for discussion purposes - will trim it
down later. Some questions about implementation details at the end.]

Zorro Lang recently ran a new test to stress single inode extent
counts now that they are no longer limited by memory allocation.
The test was simply:

# xfs_io -f -c "falloc 0 40t" /mnt/scratch/big-file
# ~/src/xfstests-dev/punch-alternating /mnt/scratch/big-file

This test uncovered a problem where the hole punching operation
appeared to finish with no error, but apparently only created 268M
extents instead of the 10 billion it was supposed to.

Further, trying to punch out extents that should have been present
resulted in success, but no change in the extent count. It looked
like a silent failure.

While running the test and observing the behaviour in real time,
I observed the extent coutn growing at ~2M extents/minute, and saw
this after about an hour:

# xfs_io -f -c "stat" /mnt/scratch/big-file |grep next ; \
> sleep 60 ; \
> xfs_io -f -c "stat" /mnt/scratch/big-file |grep next
fsxattr.nextents = 127657993
fsxattr.nextents = 129683339
#

And a few minutes later this:

# xfs_io -f -c "stat" /mnt/scratch/big-file |grep next
fsxattr.nextents = 4177861124
#

Ah, what? Where did that 4 billion extra extents suddenly come from?

Stop the workload, unmount, mount:

# xfs_io -f -c "stat" /mnt/scratch/big-file |grep next
fsxattr.nextents = 166044375
#

And it's back at the expected number. i.e. the extent count is
correct on disk, but it's screwed up in memory. I loaded up the
extent list, and immediately:

# xfs_io -f -c "stat" /mnt/scratch/big-file |grep next
fsxattr.nextents = 4192576215
#

It's bad again. So, where does that number come from?
xfs_fill_fsxattr():

                if (ip->i_df.if_flags & XFS_IFEXTENTS)
                        fa->fsx_nextents = xfs_iext_count(&ip->i_df);
                else
                        fa->fsx_nextents = ip->i_d.di_nextents;

And that's the behaviour I just saw in a nutshell. The on disk count
is correct, but once the tree is loaded into memory, it goes whacky.
Clearly there's something wrong with xfs_iext_count():

inline xfs_extnum_t xfs_iext_count(struct xfs_ifork *ifp)
{
        return ifp->if_bytes / sizeof(struct xfs_iext_rec);
}

Simple enough, but 134M extents is 2**27, and that's right about
where things went wrong. A struct xfs_iext_rec is 16 bytes in size,
which means 2**27 * 2**4 = 2**31 and we're right on target for an
integer overflow. And, sure enough:

struct xfs_ifork {
        int                     if_bytes;       /* bytes in if_u1 */
....

Once we get 2**27 extents in a file, we overflow if_bytes and the
in-core extent count goes wrong. And when we reach 2**28 extents,
if_bytes wraps back to zero and things really start to go wrong
there. This is where the silent failure comes from - only the first
2**28 extents can be looked up directly due to the overflow, all the
extents above this index wrap back to somewhere in the first 2**28
extents. Hence with a regular pattern, trying to punch a hole in the
range that didn't have holes mapped to a hole in the first 2**28
extents and so "succeeded" without changing anything. Hence "silent
failure"...

Fix this by converting if_bytes to a int64_t and converting all the
index variables and size calculations to use int64_t types to avoid
overflows in future. Signed integers are still used to enable easy
detection of extent count underflows. This enables scalability of
extent counts to the limits of the on-disk format - MAXEXTNUM
(2**31) extents.

Current testing is at over 500M extents and still going:

fsxattr.nextents = 517310478

Reported-by: Zorro Lang <zlang@redhat.com>
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
fs/xfs/libxfs/xfs_attr_leaf.c
fs/xfs/libxfs/xfs_dir2_sf.c
fs/xfs/libxfs/xfs_iext_tree.c
fs/xfs/libxfs/xfs_inode_fork.c
fs/xfs/libxfs/xfs_inode_fork.h

index f0089e8..962319e 100644 (file)
@@ -453,13 +453,15 @@ xfs_attr_copy_value(
  * special case for dev/uuid inodes, they have fixed size data forks.
  */
 int
-xfs_attr_shortform_bytesfit(xfs_inode_t *dp, int bytes)
+xfs_attr_shortform_bytesfit(
+       struct xfs_inode        *dp,
+       int                     bytes)
 {
-       int offset;
-       int minforkoff; /* lower limit on valid forkoff locations */
-       int maxforkoff; /* upper limit on valid forkoff locations */
-       int dsize;
-       xfs_mount_t *mp = dp->i_mount;
+       struct xfs_mount        *mp = dp->i_mount;
+       int64_t                 dsize;
+       int                     minforkoff;
+       int                     maxforkoff;
+       int                     offset;
 
        /* rounded down */
        offset = (XFS_LITINO(mp, dp->i_d.di_version) - bytes) >> 3;
@@ -525,7 +527,7 @@ xfs_attr_shortform_bytesfit(xfs_inode_t *dp, int bytes)
         * A data fork btree root must have space for at least
         * MINDBTPTRS key/ptr pairs if the data fork is small or empty.
         */
-       minforkoff = max(dsize, XFS_BMDR_SPACE_CALC(MINDBTPTRS));
+       minforkoff = max_t(int64_t, dsize, XFS_BMDR_SPACE_CALC(MINDBTPTRS));
        minforkoff = roundup(minforkoff, 8) >> 3;
 
        /* attr fork btree root can have at least this many key/ptr pairs */
@@ -924,7 +926,7 @@ xfs_attr_shortform_verify(
        char                            *endp;
        struct xfs_ifork                *ifp;
        int                             i;
-       int                             size;
+       int64_t                         size;
 
        ASSERT(ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL);
        ifp = XFS_IFORK_PTR(ip, XFS_ATTR_FORK);
index 85f14fc..ae16ca7 100644 (file)
@@ -628,7 +628,7 @@ xfs_dir2_sf_verify(
        int                             i;
        int                             i8count;
        int                             offset;
-       int                             size;
+       int64_t                         size;
        int                             error;
        uint8_t                         filetype;
 
index 7bc8740..5245180 100644 (file)
@@ -596,7 +596,7 @@ xfs_iext_realloc_root(
        struct xfs_ifork        *ifp,
        struct xfs_iext_cursor  *cur)
 {
-       size_t new_size = ifp->if_bytes + sizeof(struct xfs_iext_rec);
+       int64_t new_size = ifp->if_bytes + sizeof(struct xfs_iext_rec);
        void *new;
 
        /* account for the prev/next pointers */
index c643bee..8fdd042 100644 (file)
@@ -129,7 +129,7 @@ xfs_init_local_fork(
        struct xfs_inode        *ip,
        int                     whichfork,
        const void              *data,
-       int                     size)
+       int64_t                 size)
 {
        struct xfs_ifork        *ifp = XFS_IFORK_PTR(ip, whichfork);
        int                     mem_size = size, real_size = 0;
@@ -467,11 +467,11 @@ xfs_iroot_realloc(
 void
 xfs_idata_realloc(
        struct xfs_inode        *ip,
-       int                     byte_diff,
+       int64_t                 byte_diff,
        int                     whichfork)
 {
        struct xfs_ifork        *ifp = XFS_IFORK_PTR(ip, whichfork);
-       int                     new_size = (int)ifp->if_bytes + byte_diff;
+       int64_t                 new_size = ifp->if_bytes + byte_diff;
 
        ASSERT(new_size >= 0);
        ASSERT(new_size <= XFS_IFORK_SIZE(ip, whichfork));
@@ -552,7 +552,7 @@ xfs_iextents_copy(
        struct xfs_ifork        *ifp = XFS_IFORK_PTR(ip, whichfork);
        struct xfs_iext_cursor  icur;
        struct xfs_bmbt_irec    rec;
-       int                     copied = 0;
+       int64_t                 copied = 0;
 
        ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL | XFS_ILOCK_SHARED));
        ASSERT(ifp->if_bytes > 0);
index 00c62ce..7b845c0 100644 (file)
@@ -13,16 +13,16 @@ struct xfs_dinode;
  * File incore extent information, present for each of data & attr forks.
  */
 struct xfs_ifork {
-       int                     if_bytes;       /* bytes in if_u1 */
-       unsigned int            if_seq;         /* fork mod counter */
+       int64_t                 if_bytes;       /* bytes in if_u1 */
        struct xfs_btree_block  *if_broot;      /* file's incore btree root */
-       short                   if_broot_bytes; /* bytes allocated for root */
-       unsigned char           if_flags;       /* per-fork flags */
+       unsigned int            if_seq;         /* fork mod counter */
        int                     if_height;      /* height of the extent tree */
        union {
                void            *if_root;       /* extent tree root */
                char            *if_data;       /* inline file data */
        } if_u1;
+       short                   if_broot_bytes; /* bytes allocated for root */
+       unsigned char           if_flags;       /* per-fork flags */
 };
 
 /*
@@ -93,12 +93,14 @@ int         xfs_iformat_fork(struct xfs_inode *, struct xfs_dinode *);
 void           xfs_iflush_fork(struct xfs_inode *, struct xfs_dinode *,
                                struct xfs_inode_log_item *, int);
 void           xfs_idestroy_fork(struct xfs_inode *, int);
-void           xfs_idata_realloc(struct xfs_inode *, int, int);
+void           xfs_idata_realloc(struct xfs_inode *ip, int64_t byte_diff,
+                               int whichfork);
 void           xfs_iroot_realloc(struct xfs_inode *, int, int);
 int            xfs_iread_extents(struct xfs_trans *, struct xfs_inode *, int);
 int            xfs_iextents_copy(struct xfs_inode *, struct xfs_bmbt_rec *,
                                  int);
-void           xfs_init_local_fork(struct xfs_inode *, int, const void *, int);
+void           xfs_init_local_fork(struct xfs_inode *ip, int whichfork,
+                               const void *data, int64_t size);
 
 xfs_extnum_t   xfs_iext_count(struct xfs_ifork *ifp);
 void           xfs_iext_insert(struct xfs_inode *, struct xfs_iext_cursor *cur,