[2/2] xfs: widen inode delalloc block counter to 64-bits

Commit Message

Darrick J. Wong April 17, 2019, 1:39 a.m. UTC

From: Darrick J. Wong <[e-mail]>

Widen the incore inode's i_delayed_blks counter to be a 64-bit integer.
This is necessary to fix an integer overflow problem that can be
reproduced easily now that we use the counter to track blocks that are
assigned to the inode in memory but not on disk.  This includes actual
delalloc reservations as well as real extents in the COW fork that
are waiting to be remapped into the data fork.

These 'delayed mapping' blocks can easily exceed 2^32 blocks if one
creates a very large sparse file of size approximately 2^33 bytes with
one byte written every 2^23 bytes, sets a very large COW extent size
hint of 2^23 blocks, reflinks the first file into a second file, and
then writes a single byte every 2^23 blocks in the original file.

When this happens, we'll try to create approximately 1024 2^23 extent
reservations in the COW fork, which will overflow the counter and cause
problems.

Signed-off-by: Darrick J. Wong <[e-mail]>
---
 fs/xfs/xfs_inode.h |    2 +-
 fs/xfs/xfs_qm.c    |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

Comments

Allison Henderson April 17, 2019, 3:06 a.m. UTC | #1

Alrighty, looks good.

Reviewed-by: Allison Collins <[e-mail]>

On 4/16/19 6:39 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <[e-mail]>
> 
> Widen the incore inode's i_delayed_blks counter to be a 64-bit integer.
> This is necessary to fix an integer overflow problem that can be
> reproduced easily now that we use the counter to track blocks that are
> assigned to the inode in memory but not on disk.  This includes actual
> delalloc reservations as well as real extents in the COW fork that
> are waiting to be remapped into the data fork.
> 
> These 'delayed mapping' blocks can easily exceed 2^32 blocks if one
> creates a very large sparse file of size approximately 2^33 bytes with
> one byte written every 2^23 bytes, sets a very large COW extent size
> hint of 2^23 blocks, reflinks the first file into a second file, and
> then writes a single byte every 2^23 blocks in the original file.
> 
> When this happens, we'll try to create approximately 1024 2^23 extent
> reservations in the COW fork, which will overflow the counter and cause
> problems.
> 
> Signed-off-by: Darrick J. Wong <[e-mail]>
> ---
>   fs/xfs/xfs_inode.h |    2 +-
>   fs/xfs/xfs_qm.c    |    3 ++-
>   2 files changed, 3 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 87e701b638ae..558173f95a03 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -56,7 +56,7 @@ typedef struct xfs_inode {
>   	spinlock_t		i_flags_lock;	/* inode i_flags lock */
>   	/* Miscellaneous state. */
>   	unsigned long		i_flags;	/* see defined flags below */
> -	unsigned int		i_delayed_blks;	/* count of delay alloc blks */
> +	uint64_t		i_delayed_blks;	/* count of delay alloc blks */
>   
>   	struct xfs_icdinode	i_d;		/* most of ondisk inode */
>   
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index 52ed7904df10..aa6b6db3db0e 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -1812,7 +1812,8 @@ xfs_qm_vop_chown_reserve(
>   	uint			flags)
>   {
>   	struct xfs_mount	*mp = ip->i_mount;
> -	uint			delblks, blkflags, prjflags = 0;
> +	uint64_t		delblks;
> +	unsigned int		blkflags, prjflags = 0;
>   	struct xfs_dquot	*udq_unres = NULL;
>   	struct xfs_dquot	*gdq_unres = NULL;
>   	struct xfs_dquot	*pdq_unres = NULL;
>

Dave Chinner April 17, 2019, 9:10 p.m. UTC | #2

On Tue, Apr 16, 2019 at 06:39:54PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <[e-mail]>
> 
> Widen the incore inode's i_delayed_blks counter to be a 64-bit integer.
> This is necessary to fix an integer overflow problem that can be
> reproduced easily now that we use the counter to track blocks that are
> assigned to the inode in memory but not on disk.  This includes actual
> delalloc reservations as well as real extents in the COW fork that
> are waiting to be remapped into the data fork.
> 
> These 'delayed mapping' blocks can easily exceed 2^32 blocks if one
> creates a very large sparse file of size approximately 2^33 bytes with
> one byte written every 2^23 bytes, sets a very large COW extent size
> hint of 2^23 blocks, reflinks the first file into a second file, and
> then writes a single byte every 2^23 blocks in the original file.
> 
> When this happens, we'll try to create approximately 1024 2^23 extent
> reservations in the COW fork, which will overflow the counter and cause
> problems.
> 
> Signed-off-by: Darrick J. Wong <[e-mail]>
> ---
>  fs/xfs/xfs_inode.h |    2 +-
>  fs/xfs/xfs_qm.c    |    3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 87e701b638ae..558173f95a03 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -56,7 +56,7 @@ typedef struct xfs_inode {
>  	spinlock_t		i_flags_lock;	/* inode i_flags lock */
>  	/* Miscellaneous state. */
>  	unsigned long		i_flags;	/* see defined flags below */
> -	unsigned int		i_delayed_blks;	/* count of delay alloc blks */
> +	uint64_t		i_delayed_blks;	/* count of delay alloc blks */
>  
>  	struct xfs_icdinode	i_d;		/* most of ondisk inode */

This fills a 4 byte hole in the structure, yes?  Might be worth
mentioning in the commit log that it doesn't increase the size of
the struct inode, then.

Reviewed-by: Dave Chinner <[e-mail]>

Christoph Hellwig April 23, 2019, 6:28 a.m. UTC | #3

Looks good,

Reviewed-by: Christoph Hellwig <[e-mail]>

Patch

diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 87e701b638ae..558173f95a03 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -56,7 +56,7 @@  typedef struct xfs_inode {
 	spinlock_t		i_flags_lock;	/* inode i_flags lock */
 	/* Miscellaneous state. */
 	unsigned long		i_flags;	/* see defined flags below */
-	unsigned int		i_delayed_blks;	/* count of delay alloc blks */
+	uint64_t		i_delayed_blks;	/* count of delay alloc blks */
 
 	struct xfs_icdinode	i_d;		/* most of ondisk inode */
 
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 52ed7904df10..aa6b6db3db0e 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -1812,7 +1812,8 @@  xfs_qm_vop_chown_reserve(
 	uint			flags)
 {
 	struct xfs_mount	*mp = ip->i_mount;
-	uint			delblks, blkflags, prjflags = 0;
+	uint64_t		delblks;
+	unsigned int		blkflags, prjflags = 0;
 	struct xfs_dquot	*udq_unres = NULL;
 	struct xfs_dquot	*gdq_unres = NULL;
 	struct xfs_dquot	*pdq_unres = NULL;