Skip to content

Commit 3f8a4f1

Browse files
Dave Chinnerdjwong
authored andcommitted
xfs: fix inode fork extent count overflow
[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>
1 parent 4b29ab0 commit 3f8a4f1

File tree

5 files changed

+24
-20
lines changed

5 files changed

+24
-20
lines changed

fs/xfs/libxfs/xfs_attr_leaf.c

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -453,13 +453,15 @@ xfs_attr_copy_value(
453453
* special case for dev/uuid inodes, they have fixed size data forks.
454454
*/
455455
int
456-
xfs_attr_shortform_bytesfit(xfs_inode_t *dp, int bytes)
456+
xfs_attr_shortform_bytesfit(
457+
struct xfs_inode *dp,
458+
int bytes)
457459
{
458-
int offset;
459-
int minforkoff; /* lower limit on valid forkoff locations */
460-
int maxforkoff; /* upper limit on valid forkoff locations */
461-
int dsize;
462-
xfs_mount_t *mp = dp->i_mount;
460+
struct xfs_mount *mp = dp->i_mount;
461+
int64_t dsize;
462+
int minforkoff;
463+
int maxforkoff;
464+
int offset;
463465

464466
/* rounded down */
465467
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)
525527
* A data fork btree root must have space for at least
526528
* MINDBTPTRS key/ptr pairs if the data fork is small or empty.
527529
*/
528-
minforkoff = max(dsize, XFS_BMDR_SPACE_CALC(MINDBTPTRS));
530+
minforkoff = max_t(int64_t, dsize, XFS_BMDR_SPACE_CALC(MINDBTPTRS));
529531
minforkoff = roundup(minforkoff, 8) >> 3;
530532

531533
/* attr fork btree root can have at least this many key/ptr pairs */
@@ -924,7 +926,7 @@ xfs_attr_shortform_verify(
924926
char *endp;
925927
struct xfs_ifork *ifp;
926928
int i;
927-
int size;
929+
int64_t size;
928930

929931
ASSERT(ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL);
930932
ifp = XFS_IFORK_PTR(ip, XFS_ATTR_FORK);

fs/xfs/libxfs/xfs_dir2_sf.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -628,7 +628,7 @@ xfs_dir2_sf_verify(
628628
int i;
629629
int i8count;
630630
int offset;
631-
int size;
631+
int64_t size;
632632
int error;
633633
uint8_t filetype;
634634

fs/xfs/libxfs/xfs_iext_tree.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -596,7 +596,7 @@ xfs_iext_realloc_root(
596596
struct xfs_ifork *ifp,
597597
struct xfs_iext_cursor *cur)
598598
{
599-
size_t new_size = ifp->if_bytes + sizeof(struct xfs_iext_rec);
599+
int64_t new_size = ifp->if_bytes + sizeof(struct xfs_iext_rec);
600600
void *new;
601601

602602
/* account for the prev/next pointers */

fs/xfs/libxfs/xfs_inode_fork.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ xfs_init_local_fork(
129129
struct xfs_inode *ip,
130130
int whichfork,
131131
const void *data,
132-
int size)
132+
int64_t size)
133133
{
134134
struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
135135
int mem_size = size, real_size = 0;
@@ -467,11 +467,11 @@ xfs_iroot_realloc(
467467
void
468468
xfs_idata_realloc(
469469
struct xfs_inode *ip,
470-
int byte_diff,
470+
int64_t byte_diff,
471471
int whichfork)
472472
{
473473
struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
474-
int new_size = (int)ifp->if_bytes + byte_diff;
474+
int64_t new_size = ifp->if_bytes + byte_diff;
475475

476476
ASSERT(new_size >= 0);
477477
ASSERT(new_size <= XFS_IFORK_SIZE(ip, whichfork));
@@ -552,7 +552,7 @@ xfs_iextents_copy(
552552
struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
553553
struct xfs_iext_cursor icur;
554554
struct xfs_bmbt_irec rec;
555-
int copied = 0;
555+
int64_t copied = 0;
556556

557557
ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL | XFS_ILOCK_SHARED));
558558
ASSERT(ifp->if_bytes > 0);

fs/xfs/libxfs/xfs_inode_fork.h

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,16 @@ struct xfs_dinode;
1313
* File incore extent information, present for each of data & attr forks.
1414
*/
1515
struct xfs_ifork {
16-
int if_bytes; /* bytes in if_u1 */
17-
unsigned int if_seq; /* fork mod counter */
16+
int64_t if_bytes; /* bytes in if_u1 */
1817
struct xfs_btree_block *if_broot; /* file's incore btree root */
19-
short if_broot_bytes; /* bytes allocated for root */
20-
unsigned char if_flags; /* per-fork flags */
18+
unsigned int if_seq; /* fork mod counter */
2119
int if_height; /* height of the extent tree */
2220
union {
2321
void *if_root; /* extent tree root */
2422
char *if_data; /* inline file data */
2523
} if_u1;
24+
short if_broot_bytes; /* bytes allocated for root */
25+
unsigned char if_flags; /* per-fork flags */
2626
};
2727

2828
/*
@@ -93,12 +93,14 @@ int xfs_iformat_fork(struct xfs_inode *, struct xfs_dinode *);
9393
void xfs_iflush_fork(struct xfs_inode *, struct xfs_dinode *,
9494
struct xfs_inode_log_item *, int);
9595
void xfs_idestroy_fork(struct xfs_inode *, int);
96-
void xfs_idata_realloc(struct xfs_inode *, int, int);
96+
void xfs_idata_realloc(struct xfs_inode *ip, int64_t byte_diff,
97+
int whichfork);
9798
void xfs_iroot_realloc(struct xfs_inode *, int, int);
9899
int xfs_iread_extents(struct xfs_trans *, struct xfs_inode *, int);
99100
int xfs_iextents_copy(struct xfs_inode *, struct xfs_bmbt_rec *,
100101
int);
101-
void xfs_init_local_fork(struct xfs_inode *, int, const void *, int);
102+
void xfs_init_local_fork(struct xfs_inode *ip, int whichfork,
103+
const void *data, int64_t size);
102104

103105
xfs_extnum_t xfs_iext_count(struct xfs_ifork *ifp);
104106
void xfs_iext_insert(struct xfs_inode *, struct xfs_iext_cursor *cur,

0 commit comments

Comments
 (0)