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

[Yousef Alhouseen]

Yes. The KMSAN report traces the allocation through
xfs_bmap_extents_to_btree(), which calls xfs_bmap_broot_realloc(...,
1). With no existing root, that reaches krealloc(NULL, new_size). The
conversion initializes the block header, first key, and end-anchored
pointer, but does not initialize the layout/alignment gap within the
full if_broot_bytes allocation. XFS_ILOG_DBROOT later copies that
complete byte count into the log.

So the observed bytes are root-layout slack from the extents-to-btree
conversion, rather than record storage that a later insertion should
have filled.

Thanks for the review.

On Tue, 30 Jun 2026 09:11:13 -0700, "Darrick J. Wong" <djwong@xxxxxxxxxx> wrote:
> 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
> >