Re: [PATCH v2] xfs: zero newly allocated btree root space

[Darrick J. Wong]

On Tue, Jun 30, 2026 at 12:06:21PM +0200, Yousef Alhouseen wrote:
> xfs_broot_realloc() preserves the existing in-inode btree root while
> growing its allocation, but leaves the added bytes uninitialized.  The
> inode log formatter copies if_broot_bytes bytes into the journal, so those
> bytes reach the log record and its CRC calculation before every location
> has necessarily been overwritten by btree updates.
> 
> Request __GFP_ZERO for the initial allocation and every subsequent
> allocation or reallocation, as required by krealloc() semantics.  This
> keeps stale heap contents out of the filesystem log without a separate
> memset after each growth.
> 
> Fixes: 6c1c55ac3c05 ("xfs: refactor the inode fork memory allocation functions")
> Suggested-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> Reported-by: syzbot+97f2c05378c5d68dcb8c@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=97f2c05378c5d68dcb8c

I wonder, did you figure out exactly *which* code path was leaving
if_broot partially uninitialized?  Somewhere out there, someone will get
cranky at the reduced performance that comes from zeroing (especially on
krealloc) when most of the codepaths will immediately zero/set the
buffer anyway.

> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Yousef Alhouseen <alhouseenyousef@xxxxxxxxx>

In the long run I'm willing to take a small performance hit of having
many layered protections as is reasonably performant to avoid spilling
kernel memory contents to disk, though, so

Reviewed-by: "Darrick J. Wong" <djwong@xxxxxxxxxx>

(others may disagree)

--D

> ---
> Changes in v2:
> - Use __GFP_ZERO instead of an explicit memset after krealloc().
> - Apply __GFP_ZERO consistently across the allocation lifetime.
> 
>  fs/xfs/libxfs/xfs_inode_fork.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> index 606a36526ce2..dc05540fa85b 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.c
> +++ b/fs/xfs/libxfs/xfs_inode_fork.c
> @@ -384,7 +384,8 @@ xfs_broot_alloc(
>  	ASSERT(ifp->if_broot == NULL);
>  
>  	ifp->if_broot = kmalloc(new_size,
> -				GFP_KERNEL | __GFP_NOLOCKDEP | __GFP_NOFAIL);
> +				GFP_KERNEL | __GFP_NOLOCKDEP | __GFP_NOFAIL |
> +				__GFP_ZERO);
>  	ifp->if_broot_bytes = new_size;
>  	return ifp->if_broot;
>  }
> @@ -417,7 +418,8 @@ xfs_broot_realloc(
>  	if (ifp->if_broot_bytes > 0 && ifp->if_broot_bytes > new_size) {
>  		struct xfs_btree_block	*old_broot = ifp->if_broot;
>  
> -		ifp->if_broot = kmalloc(new_size, GFP_KERNEL | __GFP_NOFAIL);
> +		ifp->if_broot = kmalloc(new_size,
> +					GFP_KERNEL | __GFP_NOFAIL | __GFP_ZERO);
>  		ifp->if_broot_bytes = new_size;
>  		memcpy(ifp->if_broot, old_broot, new_size);
>  		kfree(old_broot);
> @@ -429,7 +431,7 @@ xfs_broot_realloc(
>  	 * object.
>  	 */
>  	ifp->if_broot = krealloc(ifp->if_broot, new_size,
> -			GFP_KERNEL | __GFP_NOFAIL);
> +			GFP_KERNEL | __GFP_NOFAIL | __GFP_ZERO);
>  	ifp->if_broot_bytes = new_size;
>  	return ifp->if_broot;
>  }
> -- 
> 2.54.0
>